In v2: - lots more patches to allow cross-connection plugin calls - added patch for exportname filtering - working testsuite, including bug fixes in v1 that it uncovered - more cross-references in man pages Eric Blake (6): backend: Split out new backend_handle_* functions Revert "filters: Remove most next_* wrappers" Revert "server: filters: Remove struct b_h." filters: Track next handle alongside next backend multi-conn: New filter multi-conn: Add knob to limit consistency emulation by export name docs/nbdkit-filter.pod | 4 +- filters/cache/nbdkit-cache-filter.pod | 5 +- filters/fua/nbdkit-fua-filter.pod | 7 + .../multi-conn/nbdkit-multi-conn-filter.pod | 201 ++++++++ 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 + include/nbdkit-filter.h | 145 +++--- configure.ac | 4 +- filters/multi-conn/Makefile.am | 68 +++ tests/Makefile.am | 13 +- server/internal.h | 60 ++- server/backend.c | 147 +++++- server/extents.c | 7 +- server/filters.c | 424 +++++++++++++--- filters/checkwrite/checkwrite.c | 2 +- filters/exitlast/exitlast.c | 2 +- filters/exitwhen/exitwhen.c | 2 +- filters/gzip/gzip.c | 2 +- filters/limit/limit.c | 6 +- filters/log/log.c | 2 +- filters/multi-conn/multi-conn.c | 470 ++++++++++++++++++ filters/tar/tar.c | 2 +- tests/test-multi-conn-name.sh | 88 ++++ tests/test-multi-conn-plugin.sh | 141 ++++++ tests/test-multi-conn.sh | 293 +++++++++++ TODO | 7 - 28 files changed, 1921 insertions(+), 185 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-name.sh create mode 100755 tests/test-multi-conn-plugin.sh create mode 100755 tests/test-multi-conn.sh -- 2.30.1
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 1/6] backend: Split out new backend_handle_* functions
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
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 2/6] Revert "filters: Remove most next_* wrappers"
This reverts commit 2b29d6115d67e1cc6e96c1cb17e8a7fd1b10b336. It turns out the next_* wrappers are going to be important after all in order to implement the multi-conn filter, as well as our eventual goal of allowing filters to open up a background handle into the plugin that can be shared among multiple connections into the filter. The reason is that void *nxdata will need to track the struct handle* associated with a given entry into the next layer. The backport is more complex than the original patch, due to the addition of new callbacks, as well as several filters that used the typedef nbdkit_backend instead of void*. Still, it's fairly mechanical. --- docs/nbdkit-filter.pod | 4 +- include/nbdkit-filter.h | 145 +++++++++---------- server/internal.h | 1 - server/extents.c | 7 +- server/filters.c | 247 ++++++++++++++++++++++++++++---- filters/checkwrite/checkwrite.c | 2 +- filters/exitlast/exitlast.c | 2 +- filters/exitwhen/exitwhen.c | 2 +- filters/gzip/gzip.c | 2 +- filters/limit/limit.c | 6 +- filters/log/log.c | 2 +- filters/tar/tar.c | 2 +- 12 files changed, 298 insertions(+), 124 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 98a08ca4..0f1ef8b6 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -865,7 +865,7 @@ extents covering the region C<[offset..offset+count-1]>. struct nbdkit_extents *nbdkit_extents_full ( struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, + void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err); @@ -885,7 +885,7 @@ A convenience function is provided to filters only which makes it easier to ensure that the client only encounters aligned extents. int nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, + void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, uint32_t align, struct nbdkit_extents *extents, int *err); diff --git a/include/nbdkit-filter.h b/include/nbdkit-filter.h index 0964c6e7..62efca07 100644 --- a/include/nbdkit-filter.h +++ b/include/nbdkit-filter.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 @@ -49,65 +49,56 @@ extern "C" { #define NBDKIT_ZERO_EMULATE 1 #define NBDKIT_ZERO_NATIVE 2 -#ifdef NBDKIT_INTERNAL -/* Opaque type encapsulating all information needed for calling into - * the next filter or plugin. - */ -typedef struct backend nbdkit_backend; -#else -typedef void nbdkit_backend; -#endif - /* Next ops. */ -typedef int nbdkit_next_config (nbdkit_backend *nxdata, +typedef int nbdkit_next_config (void *nxdata, const char *key, const char *value); -typedef int nbdkit_next_config_complete (nbdkit_backend *nxdata); -typedef int nbdkit_next_get_ready (nbdkit_backend *nxdata); -typedef int nbdkit_next_after_fork (nbdkit_backend *nxdata); -typedef int nbdkit_next_preconnect (nbdkit_backend *nxdata, int readonly); -typedef int nbdkit_next_list_exports (nbdkit_backend *nxdata, int readonly, +typedef int nbdkit_next_config_complete (void *nxdata); +typedef int nbdkit_next_get_ready (void *nxdata); +typedef int nbdkit_next_after_fork (void *nxdata); +typedef int nbdkit_next_preconnect (void *nxdata, int readonly); +typedef int nbdkit_next_list_exports (void *nxdata, int readonly, struct nbdkit_exports *exports); -typedef const char *nbdkit_next_default_export (nbdkit_backend *nxdata, +typedef const char *nbdkit_next_default_export (void *nxdata, int readonly); -typedef int nbdkit_next_open (nbdkit_backend *nxdata, +typedef int nbdkit_next_open (void *nxdata, int readonly, const char *exportname); struct nbdkit_next_ops { /* Performs close + open on the underlying chain. * Used by the retry filter. */ - int (*reopen) (nbdkit_backend *nxdata, int readonly, const char *exportname); + int (*reopen) (void *nxdata, int readonly, const char *exportname); /* The rest of the next ops are the same as normal plugin operations. */ - int64_t (*get_size) (nbdkit_backend *nxdata); - const char * (*export_description) (nbdkit_backend *nxdata); + int64_t (*get_size) (void *nxdata); + const char * (*export_description) (void *nxdata); - int (*can_write) (nbdkit_backend *nxdata); - int (*can_flush) (nbdkit_backend *nxdata); - int (*is_rotational) (nbdkit_backend *nxdata); - int (*can_trim) (nbdkit_backend *nxdata); - int (*can_zero) (nbdkit_backend *nxdata); - int (*can_fast_zero) (nbdkit_backend *nxdata); - int (*can_extents) (nbdkit_backend *nxdata); - int (*can_fua) (nbdkit_backend *nxdata); - int (*can_multi_conn) (nbdkit_backend *nxdata); - int (*can_cache) (nbdkit_backend *nxdata); + int (*can_write) (void *nxdata); + int (*can_flush) (void *nxdata); + int (*is_rotational) (void *nxdata); + int (*can_trim) (void *nxdata); + int (*can_zero) (void *nxdata); + int (*can_fast_zero) (void *nxdata); + int (*can_extents) (void *nxdata); + int (*can_fua) (void *nxdata); + int (*can_multi_conn) (void *nxdata); + int (*can_cache) (void *nxdata); - int (*pread) (nbdkit_backend *nxdata, + int (*pread) (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*pwrite) (nbdkit_backend *nxdata, + int (*pwrite) (void *nxdata, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*flush) (nbdkit_backend *nxdata, uint32_t flags, int *err); - int (*trim) (nbdkit_backend *nxdata, uint32_t count, uint64_t offset, - uint32_t flags, int *err); - int (*zero) (nbdkit_backend *nxdata, uint32_t count, uint64_t offset, - uint32_t flags, int *err); - int (*extents) (nbdkit_backend *nxdata, uint32_t count, uint64_t offset, - uint32_t flags, struct nbdkit_extents *extents, int *err); - int (*cache) (nbdkit_backend *nxdata, uint32_t count, uint64_t offset, - uint32_t flags, int *err); + int (*flush) (void *nxdata, uint32_t flags, int *err); + int (*trim) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, + int *err); + int (*zero) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, + int *err); + int (*extents) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents *extents, int *err); + int (*cache) (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, + int *err); }; /* Extent functions. */ @@ -117,7 +108,7 @@ struct nbdkit_extent { uint32_t type; }; -NBDKIT_EXTERN_DECL (struct nbdkit_extents *,nbdkit_extents_new, +NBDKIT_EXTERN_DECL (struct nbdkit_extents *, nbdkit_extents_new, (uint64_t start, uint64_t end)); NBDKIT_EXTERN_DECL (void, nbdkit_extents_free, (struct nbdkit_extents *)); NBDKIT_EXTERN_DECL (size_t, nbdkit_extents_count, @@ -126,12 +117,12 @@ NBDKIT_EXTERN_DECL (struct nbdkit_extent, nbdkit_get_extent, (const struct nbdkit_extents *, size_t)); NBDKIT_EXTERN_DECL (struct nbdkit_extents *, nbdkit_extents_full, (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, + void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err)); NBDKIT_EXTERN_DECL (int, nbdkit_extents_aligned, (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, + void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, uint32_t align, struct nbdkit_extents *extents, int *err)); @@ -172,78 +163,78 @@ struct nbdkit_filter { void (*load) (void); void (*unload) (void); - int (*config) (nbdkit_next_config *next, nbdkit_backend *nxdata, + int (*config) (nbdkit_next_config *next, void *nxdata, const char *key, const char *value); - int (*config_complete) (nbdkit_next_config_complete *next, - nbdkit_backend *nxdata); + int (*config_complete) (nbdkit_next_config_complete *next, void *nxdata); const char *config_help; int (*thread_model) (void); - int (*get_ready) (nbdkit_next_get_ready *next, nbdkit_backend *nxdata, + int (*get_ready) (nbdkit_next_get_ready *next, void *nxdata, int thread_model); - int (*after_fork) (nbdkit_next_after_fork *next, nbdkit_backend *nxdata); - int (*preconnect) (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, + int (*after_fork) (nbdkit_next_after_fork *next, void *nxdata); + int (*preconnect) (nbdkit_next_preconnect *next, void *nxdata, int readonly); - int (*list_exports) (nbdkit_next_list_exports *next, nbdkit_backend *nxdata, + int (*list_exports) (nbdkit_next_list_exports *next, void *nxdata, int readonly, int is_tls, struct nbdkit_exports *exports); const char * (*default_export) (nbdkit_next_default_export *next, - nbdkit_backend *nxdata, + void *nxdata, int readonly, int is_tls); - void * (*open) (nbdkit_next_open *next, nbdkit_backend *nxdata, + void * (*open) (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls); void (*close) (void *handle); - int (*prepare) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*prepare) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, int readonly); - int (*finalize) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*finalize) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int64_t (*get_size) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int64_t (*get_size) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); const char * (*export_description) (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, void *handle); + void *nxdata, void *handle); - int (*can_write) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_write) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*can_flush) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_flush) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); int (*is_rotational) (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, void *handle); - int (*can_trim) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + void *nxdata, + void *handle); + int (*can_trim) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*can_zero) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_zero) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*can_fast_zero) (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, void *handle); - int (*can_extents) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_fast_zero) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle); + int (*can_extents) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*can_fua) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_fua) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, void *handle); - int (*can_cache) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*can_multi_conn) (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle); + int (*can_cache) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle); - int (*pread) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*pread) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*pwrite) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*pwrite) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*flush) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*flush) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t flags, int *err); - int (*trim) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*trim) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*zero) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*zero) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); - int (*extents) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*extents) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err); - int (*cache) (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, + int (*cache) (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err); }; diff --git a/server/internal.h b/server/internal.h index b999b856..06105d79 100644 --- a/server/internal.h +++ b/server/internal.h @@ -43,7 +43,6 @@ #endif #define NBDKIT_API_VERSION 2 -#define NBDKIT_INTERNAL #include "nbdkit-plugin.h" #include "nbdkit-filter.h" #include "cleanup.h" diff --git a/server/extents.c b/server/extents.c index cbc0f0dd..a509bc94 100644 --- a/server/extents.c +++ b/server/extents.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2019-2020 Red Hat Inc. + * Copyright (C) 2019-2021 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -212,8 +212,7 @@ nbdkit_add_extent (struct nbdkit_extents *exts, /* Compute aligned extents on behalf of a filter. */ int -nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, - nbdkit_backend *nxdata, +nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, uint32_t align, struct nbdkit_extents *exts, int *err) @@ -298,7 +297,7 @@ nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, * covering the region [offset..offset+count-1]. */ struct nbdkit_extents * -nbdkit_extents_full (struct nbdkit_next_ops *next_ops, nbdkit_backend *nxdata, +nbdkit_extents_full (struct nbdkit_next_ops *next_ops, void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { diff --git a/server/filters.c b/server/filters.c index 54abf9a4..f4dbfea7 100644 --- a/server/filters.c +++ b/server/filters.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 @@ -127,8 +127,9 @@ filter_dump_fields (struct backend *b) } static int -next_config (struct backend *b, const char *key, const char *value) +next_config (void *nxdata, const char *key, const char *value) { + struct backend *b = nxdata; b->config (b, key, value); return 0; } @@ -150,8 +151,9 @@ filter_config (struct backend *b, const char *key, const char *value) } static int -next_config_complete (struct backend *b) +next_config_complete (void *nxdata) { + struct backend *b = nxdata; b->config_complete (b); return 0; } @@ -172,9 +174,10 @@ filter_config_complete (struct backend *b) } static int -next_get_ready (struct backend *b) +next_get_ready (void *nxdata) { - b->get_ready (b); + struct backend *b_next = nxdata; + b_next->get_ready (b_next); return 0; } @@ -194,9 +197,10 @@ filter_get_ready (struct backend *b) } static int -next_after_fork (struct backend *b) +next_after_fork (void *nxdata) { - b->after_fork (b); + struct backend *b_next = nxdata; + b_next->after_fork (b_next); return 0; } @@ -215,6 +219,13 @@ filter_after_fork (struct backend *b) b->next->after_fork (b->next); } +static int +next_preconnect (void *nxdata, int readonly) +{ + struct backend *b_next = nxdata; + return b_next->preconnect (b_next, readonly); +} + static int filter_preconnect (struct backend *b, int readonly) { @@ -223,7 +234,7 @@ filter_preconnect (struct backend *b, int readonly) debug ("%s: preconnect", b->name); if (f->filter.preconnect) - return f->filter.preconnect (b->next->preconnect, b->next, readonly); + return f->filter.preconnect (next_preconnect, b->next, readonly); else return b->next->preconnect (b->next, readonly); } @@ -237,6 +248,13 @@ plugin_magic_config_key (struct backend *b) return b->next->magic_config_key (b->next); } +static int +next_list_exports (void *nxdata, int readonly, struct nbdkit_exports *exports) +{ + struct backend *b_next = nxdata; + return backend_list_exports (b_next, readonly, exports); +} + static int filter_list_exports (struct backend *b, int readonly, int is_tls, struct nbdkit_exports *exports) @@ -244,22 +262,37 @@ filter_list_exports (struct backend *b, int readonly, int is_tls, struct backend_filter *f = container_of (b, struct backend_filter, backend); if (f->filter.list_exports) - return f->filter.list_exports (backend_list_exports, b->next, + return f->filter.list_exports (next_list_exports, b->next, readonly, is_tls, exports); return backend_list_exports (b->next, readonly, exports); } +static const char * +next_default_export (void *nxdata, int readonly) +{ + struct backend *b_next = nxdata; + return backend_default_export (b_next, readonly); +} + static const char * filter_default_export (struct backend *b, int readonly, int is_tls) { struct backend_filter *f = container_of (b, struct backend_filter, backend); if (f->filter.default_export) - return f->filter.default_export (backend_default_export, b->next, + return f->filter.default_export (next_default_export, b->next, readonly, is_tls); return backend_default_export (b->next, readonly); } +static int +next_open (void *nxdata, int readonly, const char *exportname) +{ + struct backend *b_next = nxdata; + + return backend_open (b_next, readonly, exportname); +} + static void * filter_open (struct backend *b, int readonly, const char *exportname, int is_tls) @@ -271,7 +304,7 @@ filter_open (struct backend *b, int readonly, const char *exportname, * inner-to-outer ordering. */ if (f->filter.open) - handle = f->filter.open (backend_open, b->next, readonly, exportname, + handle = f->filter.open (next_open, b->next, readonly, exportname, is_tls); else if (backend_open (b->next, readonly, exportname) == -1) handle = NULL; @@ -289,27 +322,179 @@ filter_close (struct backend *b, void *handle) f->filter.close (handle); } +/* 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. + */ + +static int +next_reopen (void *nxdata, int readonly, const char *exportname) +{ + struct backend *b_next = nxdata; + return backend_reopen (b_next, readonly, exportname); +} + +static int64_t +next_get_size (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_get_size (b_next); +} + +static const char * +next_export_description (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_export_description (b_next); +} + +static int +next_can_write (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_write (b_next); +} + +static int +next_can_flush (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_flush (b_next); +} + +static int +next_is_rotational (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_is_rotational (b_next); +} + +static int +next_can_trim (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_trim (b_next); +} + +static int +next_can_zero (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_zero (b_next); +} + +static int +next_can_fast_zero (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_fast_zero (b_next); +} + +static int +next_can_extents (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_extents (b_next); +} + +static int +next_can_fua (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_fua (b_next); +} + +static int +next_can_multi_conn (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_multi_conn (b_next); +} + +static int +next_can_cache (void *nxdata) +{ + struct backend *b_next = nxdata; + return backend_can_cache (b_next); +} + +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); +} + +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); +} + +static int +next_flush (void *nxdata, uint32_t flags, int *err) +{ + struct backend *b_next = nxdata; + return backend_flush (b_next, 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); +} + +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); +} + +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); +} + +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); +} + static struct nbdkit_next_ops next_ops = { - .reopen = backend_reopen, - .export_description = backend_export_description, - .get_size = backend_get_size, - .can_write = backend_can_write, - .can_flush = backend_can_flush, - .is_rotational = backend_is_rotational, - .can_trim = backend_can_trim, - .can_zero = backend_can_zero, - .can_fast_zero = backend_can_fast_zero, - .can_extents = backend_can_extents, - .can_fua = backend_can_fua, - .can_multi_conn = backend_can_multi_conn, - .can_cache = backend_can_cache, - .pread = backend_pread, - .pwrite = backend_pwrite, - .flush = backend_flush, - .trim = backend_trim, - .zero = backend_zero, - .extents = backend_extents, - .cache = backend_cache, + .reopen = next_reopen, + .export_description = next_export_description, + .get_size = next_get_size, + .can_write = next_can_write, + .can_flush = next_can_flush, + .is_rotational = next_is_rotational, + .can_trim = next_can_trim, + .can_zero = next_can_zero, + .can_fast_zero = next_can_fast_zero, + .can_extents = next_can_extents, + .can_fua = next_can_fua, + .can_multi_conn = next_can_multi_conn, + .can_cache = next_can_cache, + .pread = next_pread, + .pwrite = next_pwrite, + .flush = next_flush, + .trim = next_trim, + .zero = next_zero, + .extents = next_extents, + .cache = next_cache, }; static int diff --git a/filters/checkwrite/checkwrite.c b/filters/checkwrite/checkwrite.c index bbd7f1cd..99ba2845 100644 --- a/filters/checkwrite/checkwrite.c +++ b/filters/checkwrite/checkwrite.c @@ -44,7 +44,7 @@ #include "minmax.h" static void * -checkwrite_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +checkwrite_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { /* Ignore readonly flag passed in, open the plugin readonly. */ diff --git a/filters/exitlast/exitlast.c b/filters/exitlast/exitlast.c index 4c879cc9..c16b3948 100644 --- a/filters/exitlast/exitlast.c +++ b/filters/exitlast/exitlast.c @@ -50,7 +50,7 @@ static _Atomic unsigned connections; static void * -exitlast_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +exitlast_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { if (next (nxdata, readonly, exportname) == -1) diff --git a/filters/exitwhen/exitwhen.c b/filters/exitwhen/exitwhen.c index ca199a8c..bd1a4082 100644 --- a/filters/exitwhen/exitwhen.c +++ b/filters/exitwhen/exitwhen.c @@ -482,7 +482,7 @@ exitwhen_preconnect (nbdkit_next_preconnect *next, void *nxdata, int readonly) } static void * -exitwhen_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +exitwhen_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { if (next (nxdata, readonly, exportname) == -1) diff --git a/filters/gzip/gzip.c b/filters/gzip/gzip.c index 03e023e5..71c703e6 100644 --- a/filters/gzip/gzip.c +++ b/filters/gzip/gzip.c @@ -75,7 +75,7 @@ gzip_thread_model (void) } static void * -gzip_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +gzip_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { /* Always pass readonly=1 to the underlying plugin. */ diff --git a/filters/limit/limit.c b/filters/limit/limit.c index fb862df7..f750b235 100644 --- a/filters/limit/limit.c +++ b/filters/limit/limit.c @@ -49,7 +49,7 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; static unsigned limit = 1; static int -limit_config (nbdkit_next_config *next, nbdkit_backend *nxdata, +limit_config (nbdkit_next_config *next, void *nxdata, const char *key, const char *value) { if (strcmp (key, "limit") == 0) { @@ -73,7 +73,7 @@ too_many_clients_error (void) * out between preconnect and open. */ static int -limit_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, +limit_preconnect (nbdkit_next_preconnect *next, void *nxdata, int readonly) { if (next (nxdata, readonly) == -1) @@ -90,7 +90,7 @@ limit_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, } static void * -limit_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +limit_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { if (next (nxdata, readonly, exportname) == -1) diff --git a/filters/log/log.c b/filters/log/log.c index 7b81a8f1..8930fff3 100644 --- a/filters/log/log.c +++ b/filters/log/log.c @@ -187,7 +187,7 @@ log_list_exports (nbdkit_next_list_exports *next, void *nxdata, } static int -log_preconnect (nbdkit_next_preconnect *next, nbdkit_backend *nxdata, +log_preconnect (nbdkit_next_preconnect *next, void *nxdata, int readonly) { static log_id_t id; diff --git a/filters/tar/tar.c b/filters/tar/tar.c index fa4b61dc..e4c5913e 100644 --- a/filters/tar/tar.c +++ b/filters/tar/tar.c @@ -112,7 +112,7 @@ struct handle { }; static void * -tar_open (nbdkit_next_open *next, nbdkit_backend *nxdata, +tar_open (nbdkit_next_open *next, void *nxdata, int readonly, const char *exportname, int is_tls) { struct handle *h; -- 2.30.1
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 3/6] Revert "server: filters: Remove struct b_h."
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
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 4/6] filters: Track next handle alongside next backend
The next patch will introduce a new multi-conn filter, which needs to track all client connections in order to coordinate a flush across each open handle into the plugin. We already guarantee that next_ops and nxdata have a lifetime that is stable as long as the connection exists. However, if next_ops->flush merely calls backend_flush(b_next), it will not do what we want, because backend_flush starts with GET_CONN and grabs the handle associated with the current client connection. Instead, we want to call backend_handle_flush(b_next, h) with the handle into the plugin learned at the time that other connection was opened. The solution is obvious: add a field to our struct b_h which tracks the appropriate plugin handle on .open and .reopen, so that all filter calls through next_ops are now independent of the current connection that called into the plugin. This will also make it easier for a future patch to add the ability for a filter to open a single background context into the plugin which gets shared among all connections into the filter. The patch is fairly mechanical. --- server/filters.c | 98 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 74 insertions(+), 24 deletions(-) diff --git a/server/filters.c b/server/filters.c index 3e79ef5e..975053d4 100644 --- a/server/filters.c +++ b/server/filters.c @@ -49,13 +49,15 @@ struct backend_filter { struct nbdkit_filter filter; }; -/* Literally a backend and the filter's handle. +/* Literally a backend, the context for calling into that 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; + struct handle *h; void *handle; }; @@ -306,6 +308,7 @@ static void * filter_open (struct backend *b, int readonly, const char *exportname, int is_tls) { + GET_CONN; struct backend_filter *f = container_of (b, struct backend_filter, backend); struct b_h *nxdata = calloc (1, sizeof *nxdata); @@ -331,6 +334,8 @@ filter_open (struct backend *b, int readonly, const char *exportname, return NULL; } + nxdata->h = get_handle (conn, b->i-1); + return nxdata; } @@ -357,92 +362,123 @@ filter_close (struct backend *b, void *handle) static int next_reopen (void *nxdata, int readonly, const char *exportname) { + GET_CONN; struct b_h *b_h = nxdata; - return backend_reopen (b_h->b, readonly, exportname); + + if (backend_reopen (b_h->b, readonly, exportname) == -1) { + b_h->h = NULL; + return -1; + } + b_h->h = get_handle (conn, b_h->b->i); + return 0; } static int64_t next_get_size (void *nxdata) { struct b_h *b_h = nxdata; - return backend_get_size (b_h->b); + + assert (b_h->h); + return backend_handle_get_size (b_h->b, b_h->h); } static const char * next_export_description (void *nxdata) { struct b_h *b_h = nxdata; - return backend_export_description (b_h->b); + + assert (b_h->h); + return backend_handle_export_description (b_h->b, b_h->h); } static int next_can_write (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_write (b_h->b); + + assert (b_h->h); + return backend_handle_can_write (b_h->b, b_h->h); } static int next_can_flush (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_flush (b_h->b); + + assert (b_h->h); + return backend_handle_can_flush (b_h->b, b_h->h); } static int next_is_rotational (void *nxdata) { struct b_h *b_h = nxdata; - return backend_is_rotational (b_h->b); + + assert (b_h->h); + return backend_handle_is_rotational (b_h->b, b_h->h); } static int next_can_trim (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_trim (b_h->b); + + assert (b_h->h); + return backend_handle_can_trim (b_h->b, b_h->h); } static int next_can_zero (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_zero (b_h->b); + + assert (b_h->h); + return backend_handle_can_zero (b_h->b, b_h->h); } static int next_can_fast_zero (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_fast_zero (b_h->b); + + assert (b_h->h); + return backend_handle_can_fast_zero (b_h->b, b_h->h); } static int next_can_extents (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_extents (b_h->b); + + assert (b_h->h); + return backend_handle_can_extents (b_h->b, b_h->h); } static int next_can_fua (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_fua (b_h->b); + + assert (b_h->h); + return backend_handle_can_fua (b_h->b, b_h->h); } static int next_can_multi_conn (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_multi_conn (b_h->b); + + assert (b_h->h); + return backend_handle_can_multi_conn (b_h->b, b_h->h); } static int next_can_cache (void *nxdata) { struct b_h *b_h = nxdata; - return backend_can_cache (b_h->b); + + assert (b_h->h); + return backend_handle_can_cache (b_h->b, b_h->h); } static int @@ -450,8 +486,10 @@ next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_pread (b_h->b, buf, count, offset, flags, - err); + + assert (b_h->h); + return backend_handle_pread (b_h->b, b_h->h, buf, count, offset, flags, + err); } static int @@ -459,15 +497,19 @@ next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_pwrite (b_h->b, buf, count, offset, flags, - err); + + assert (b_h->h); + return backend_handle_pwrite (b_h->b, b_h->h, buf, count, offset, flags, + err); } static int next_flush (void *nxdata, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_flush (b_h->b, flags, err); + + assert (b_h->h); + return backend_handle_flush (b_h->b, b_h->h, flags, err); } static int @@ -475,7 +517,9 @@ next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_trim (b_h->b, count, offset, flags, err); + + assert (b_h->h); + return backend_handle_trim (b_h->b, b_h->h, count, offset, flags, err); } static int @@ -483,7 +527,9 @@ next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_zero (b_h->b, count, offset, flags, err); + + assert (b_h->h); + return backend_handle_zero (b_h->b, b_h->h, count, offset, flags, err); } static int @@ -491,8 +537,10 @@ next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { struct b_h *b_h = nxdata; - return backend_extents (b_h->b, count, offset, flags, - extents, err); + + assert (b_h->h); + return backend_handle_extents (b_h->b, b_h->h, count, offset, flags, + extents, err); } static int @@ -500,7 +548,9 @@ next_cache (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct b_h *b_h = nxdata; - return backend_cache (b_h->b, count, offset, flags, err); + + assert (b_h->h); + return backend_handle_cache (b_h->b, b_h->h, count, offset, flags, err); } static struct nbdkit_next_ops next_ops = { -- 2.30.1
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 5/6] multi-conn: New filter
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
Eric Blake
2021-Feb-25 20:59 UTC
[Libguestfs] [nbdkit PATCH v2 6/6] multi-conn: Add knob to limit consistency emulation by export name
We can't enable the knob by default since many nbdkit plugins don't care about export name, but it is certainly more efficient to limit consistency to just clients visiting the same export. Thankfully, NBD_CMD_FLUSH does not take size and offset, and therefore is safe (although wasting CPU cycles) to use even when a different export has a different size. --- .../multi-conn/nbdkit-multi-conn-filter.pod | 29 +++++- tests/Makefile.am | 2 + filters/multi-conn/multi-conn.c | 31 ++++++- tests/test-multi-conn-name.sh | 88 +++++++++++++++++++ tests/test-multi-conn-plugin.sh | 63 ++++++++----- 5 files changed, 186 insertions(+), 27 deletions(-) create mode 100755 tests/test-multi-conn-name.sh diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod index d624a6fb..a4842087 100644 --- a/filters/multi-conn/nbdkit-multi-conn-filter.pod +++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod @@ -4,8 +4,8 @@ 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...] + nbdkit --filter=multi-conn plugin [multi-conn-mode=MODE] \ + [multi-conn-track-dirty=LEVEL] [multi-conn-exportname=BOOL] [plugin-args...] =head1 DESCRIPTION @@ -26,7 +26,8 @@ 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. +a plugin that produces completely independent data per connection from +the same export name. Additional control over the behavior of client flush commands is possible by combining this filter with L<nbdkit-fua-filter(1)>. Note @@ -125,6 +126,27 @@ 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. +=item B<multi-conn-exportname=false> + +The exportname switch defaults to false for safety, and causes the +filter to flush across all active connections regardless of the export +name in use by that connection when doing emulation. However, when a +plugin supports distinct data according to export name, this behavior +will penalize the performance of clients visiting an unrelated export +by spending time on replicated flush operations not actually relevant +to that export. + +=item B<multi-conn-exportname=true> + +Setting the exportname switch to true causes the filter to only +synchronize flushes to connections visiting the same export name. +This avoids penalizing clients visiting an unrelated export name (such +as L<nbdkit-file-plugin(1)> in B<dir=> mode), but is unsafe when used +with a plugin that serves shared content across all connections +regardless of the export name requested by the client, if that plugin +is not already multi-conn consistent (such as +L<nbdkit-vddk-plugin(1)>). + =back =head1 EXAMPLES @@ -161,6 +183,7 @@ C<nbdkit-multi-conn-filter> first appeared in nbdkit 1.26. L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, +L<nbdkit-vddk-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-cache-filter(1)>, L<nbdkit-fua-filter(1)>, diff --git a/tests/Makefile.am b/tests/Makefile.am index 4b3ee65c..6df0b490 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1541,10 +1541,12 @@ EXTRA_DIST += \ # multi-conn filter test. TESTS += \ test-multi-conn.sh \ + test-multi-conn-name.sh \ $(NULL) EXTRA_DIST += \ test-multi-conn-plugin.sh \ test-multi-conn.sh \ + test-multi-conn-name.sh \ $(NULL) # nofilter test. diff --git a/filters/multi-conn/multi-conn.c b/filters/multi-conn/multi-conn.c index ce01b488..3dc8d5c7 100644 --- a/filters/multi-conn/multi-conn.c +++ b/filters/multi-conn/multi-conn.c @@ -60,6 +60,8 @@ static enum TrackDirtyMode { OFF, } track; +static bool byname = false; + enum dirty { WRITE = 1, /* A write may have populated a cache */ READ = 2, /* A read may have populated a cache */ @@ -72,6 +74,7 @@ static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; struct handle { struct nbdkit_next_ops *next_ops; void *nxdata; + char *name; enum MultiConnMode mode; /* Runtime resolution of mode==AUTO */ enum dirty dirty; /* What aspects of this connection are dirty */ }; @@ -115,6 +118,16 @@ multi_conn_config (nbdkit_next_config *next, void *nxdata, } return 0; } + else if (strcmp (key, "multi-conn-exportname") == 0 || + strcmp (key, "multi-conn-export-name") == 0) { + int r; + + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + byname = r; + return 0; + } return next (nxdata, key, value); } @@ -122,6 +135,7 @@ multi_conn_config (nbdkit_next_config *next, void *nxdata, "multi-conn-mode=<MODE> 'auto' (default), 'emulate', 'plugin',\n" \ " 'disable', or 'unsafe'.\n" \ "multi-conn-track-dirty=<LEVEL> 'conn' (default), 'fast', or 'off'.\n" + "multi-conn-exportname=<BOOL> true to limit emulation by export name.\n" static int multi_conn_get_ready (nbdkit_next_get_ready *next, void *nxdata, @@ -148,6 +162,13 @@ multi_conn_open (nbdkit_next_open *next, void *nxdata, nbdkit_error ("calloc: %m"); return NULL; } + if (byname) { + h->name = strdup (exportname); + if (h->name == NULL) { + nbdkit_error ("strdup: %m"); + return NULL; + } + } return h; } @@ -204,7 +225,10 @@ multi_conn_finalize (struct nbdkit_next_ops *next_ops, void *nxdata, static void multi_conn_close (void *handle) { - free (handle); + struct handle *h = handle; + + free (h->name); + free (h); } static int @@ -381,6 +405,8 @@ multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); for (i = 0; i < conns.size; i++) { h2 = conns.ptr[i]; + if (byname && strcmp (h->name, h2->name) != 0) + continue; if (track == OFF || (dirty && (track == FAST || h2->dirty & READ)) || h2->dirty & WRITE) { if (h2->next_ops->flush (h2->nxdata, flags, err) == -1) @@ -402,7 +428,8 @@ multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, 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; + if (!byname || strcmp (h->name, conns.ptr[i]->name) == 0) + conns.ptr[i]->dirty = 0; dirty = 0; } else diff --git a/tests/test-multi-conn-name.sh b/tests/test-multi-conn-name.sh new file mode 100755 index 00000000..698d386e --- /dev/null +++ b/tests/test-multi-conn-name.sh @@ -0,0 +1,88 @@ +#!/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 effect of multi-conn-exportname config flag + +source ./functions.sh +set -e +set -x + +requires_plugin sh +requires_nbdsh_uri +requires nbdsh -c 'print(h.set_opt_mode)' +requires dd iflag=count_bytes </dev/null + +files="test-multi-conn-name.out" +rm -f $files +cleanup_fn rm -f $files + +fail=0 +export script=' +import os + +uri = os.environ["uri"] +h = {} +for name in ["a", "b"]: + for conn in [1, 2]: + key = "%s%d" % (name, conn) + h[key] = nbd.NBD() + h[key].set_opt_mode(True) + h[key].connect_uri(uri) + h[key].set_export_name(name) + h[key].opt_go() +h["a1"].pread(1, 0) +h["a2"].pwrite(b"A", 0) +h["b1"].pread(1, 0) +h["b2"].pwrite(b"B", 0, nbd.CMD_FLAG_FUA) +print(h["a1"].pread(1, 0)) +print(h["b1"].pread(1, 0)) +' + +# Without the knob we flush all exports +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --run 'export uri; nbdsh -c "$script"' > test-multi-conn-name.out || fail=1 +diff -u <(cat <<\EOF +b'A' +b'B' +EOF + ) test-multi-conn-name.out || fail=1 +# But with the knob, our flush is specific to the correct export +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + multi-conn-exportname=true \ + --run 'export uri; nbdsh -c "$script"' > test-multi-conn-name.out || fail=1 +diff -u <(cat <<\EOF +b'I' +b'B' +EOF + ) test-multi-conn-name.out || fail=1 + +exit $fail diff --git a/tests/test-multi-conn-plugin.sh b/tests/test-multi-conn-plugin.sh index 7262a348..5d627a07 100755 --- a/tests/test-multi-conn-plugin.sh +++ b/tests/test-multi-conn-plugin.sh @@ -39,30 +39,36 @@ # it ever sends overlapping writes without coordinating flushes and still # expects any particular write to occur last). +get_export() { + case $1 in + */*) export="$tmpdir/$(dirname $1)" conn=$(basename $1) ;; + *) export="$tmpdir" conn=$1 ;; + esac +} fill_cache() { - if test ! -f "$tmpdir/$1"; then - cp "$tmpdir/0" "$tmpdir/$1" + if test ! -f "$export/$conn"; then + cp "$export/0" "$export/$conn" fi } do_fua() { - case ,$4, in + case ,$3, in *,fua,*) if test -f "$tmpdir/strictfua"; then - dd of="$tmpdir/0" if="$tmpdir/$1" skip=$3 seek=$3 count=$2 \ + dd of="$export/0" if="$export/$conn" skip=$2 seek=$2 count=$1 \ conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes else - do_flush $1 + do_flush fi ;; esac } do_flush() { - if test -f "$tmpdir/$1-replay"; then + if test -f "$export/$conn-replay"; then while read cnt off; do - dd of="$tmpdir/0" if="$tmpdir/$1" skip=$off seek=$off count=$cnt \ + dd of="$export/0" if="$export/$conn" skip=$off seek=$off count=$cnt \ conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes - done < "$tmpdir/$1-replay" + done < "$export/$conn-replay" fi - rm -f "$tmpdir/$1" "$tmpdir/$1-replay" + rm -f "$export/$conn" "$export/$conn-replay" } case "$1" in config) @@ -91,30 +97,43 @@ case "$1" in ;; open) read i < "$tmpdir/counter" - echo $((i+1)) | tee "$tmpdir/counter" + i=$((i+1)) + echo $i > "$tmpdir/counter" + if test -z "$3"; then + echo $i + else + mkdir -p "$tmpdir/$3" || exit 1 + cp "$tmpdir/0" "$tmpdir/$3/0" + echo "$3/$i" + fi ;; pread) - fill_cache $2 - dd if="$tmpdir/$2" skip=$4 count=$3 iflag=count_bytes,skip_bytes + get_export $2 + fill_cache + dd if="$export/$conn" skip=$4 count=$3 iflag=count_bytes,skip_bytes ;; cache) - fill_cache $2 + get_export $2 + fill_cache ;; 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 + get_export $2 + fill_cache + dd of="$export/$conn" seek=$4 conv=notrunc oflag=seek_bytes + echo $3 $4 >> "$export/$conn-replay" + do_fua $3 $4 $5 ;; zero | trim) - fill_cache $2 - dd of="$tmpdir/$2" if="/dev/zero" count=$3 seek=$4 conv=notrunc\ + get_export $2 + fill_cache + dd of="$export/$conn" 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 + echo $3 $4 >> "$export/$conn-replay" + do_fua $3 $4 $5 ;; flush) - do_flush $2 + get_export $2 + do_flush ;; *) exit 2 -- 2.30.1
Eric Blake
2021-Mar-01 15:23 UTC
[Libguestfs] [nbdkit PATCH v2.2 6/6] multi-conn: Add knob to limit consistency emulation by export name
We can't enable the knob by default since many nbdkit plugins don't care about export name, but it is certainly more efficient to limit consistency to just clients visiting the same export. Thankfully, NBD_CMD_FLUSH does not take size and offset, and therefore is safe (although wasting CPU cycles) to use even when a different export has a different size. --- Respinning just this patch to fix the flaw I noted earlier about not properly tracking which connections are dirty when filtering by gruop name. .../multi-conn/nbdkit-multi-conn-filter.pod | 29 ++++- tests/Makefile.am | 2 + filters/multi-conn/multi-conn.c | 118 +++++++++++++++--- tests/test-multi-conn-name.sh | 88 +++++++++++++ tests/test-multi-conn-plugin.sh | 63 ++++++---- 5 files changed, 256 insertions(+), 44 deletions(-) create mode 100755 tests/test-multi-conn-name.sh diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod index 007c4e3a..efeba570 100644 --- a/filters/multi-conn/nbdkit-multi-conn-filter.pod +++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod @@ -4,8 +4,8 @@ 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...] + nbdkit --filter=multi-conn plugin [multi-conn-mode=MODE] \ + [multi-conn-track-dirty=LEVEL] [multi-conn-exportname=BOOL] [plugin-args...] =head1 DESCRIPTION @@ -26,7 +26,8 @@ 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. +a plugin that produces completely independent data per connection from +the same export name. Additional control over the behavior of client flush commands is possible by combining this filter with L<nbdkit-fua-filter(1)>. Note @@ -125,6 +126,27 @@ 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. +=item B<multi-conn-exportname=false> + +The exportname switch defaults to false for safety, and causes the +filter to flush across all active connections regardless of the export +name in use by that connection when doing emulation. However, when a +plugin supports distinct data according to export name, this behavior +will penalize the performance of clients visiting an unrelated export +by spending time on replicated flush operations not actually relevant +to that export. + +=item B<multi-conn-exportname=true> + +Setting the exportname switch to true causes the filter to only +synchronize flushes to connections visiting the same export name. +This avoids penalizing clients visiting an unrelated export name (such +as L<nbdkit-file-plugin(1)> in B<dir=> mode), but is unsafe when used +with a plugin that serves shared content across all connections +regardless of the export name requested by the client, if that plugin +is not already multi-conn consistent (such as +L<nbdkit-vddk-plugin(1)>). + =back =head1 EXAMPLES @@ -161,6 +183,7 @@ C<nbdkit-multi-conn-filter> first appeared in nbdkit 1.26. L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, +L<nbdkit-vddk-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-cache-filter(1)>, L<nbdkit-fua-filter(1)>, diff --git a/tests/Makefile.am b/tests/Makefile.am index 4b3ee65c..6df0b490 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1541,10 +1541,12 @@ EXTRA_DIST += \ # multi-conn filter test. TESTS += \ test-multi-conn.sh \ + test-multi-conn-name.sh \ $(NULL) EXTRA_DIST += \ test-multi-conn-plugin.sh \ test-multi-conn.sh \ + test-multi-conn-name.sh \ $(NULL) # nofilter test. diff --git a/filters/multi-conn/multi-conn.c b/filters/multi-conn/multi-conn.c index d5459410..2335e13f 100644 --- a/filters/multi-conn/multi-conn.c +++ b/filters/multi-conn/multi-conn.c @@ -60,6 +60,8 @@ static enum TrackDirtyMode { OFF, } track; +static bool byname = false; + enum dirty { WRITE = 1, /* A write may have populated a cache */ READ = 2, /* A read may have populated a cache */ @@ -74,12 +76,21 @@ struct handle { void *nxdata; enum MultiConnMode mode; /* Runtime resolution of mode==AUTO */ enum dirty dirty; /* What aspects of this connection are dirty */ + char *name; /* Used when byname==true to assign group */ + struct group *group; /* All connections grouped with this one */ }; DEFINE_VECTOR_TYPE(conns_vector, struct handle *); -static conns_vector conns = empty_vector; -static bool dirty; /* True if any connection is dirty */ +struct group { + conns_vector conns; + char *name; + bool dirty; /* True if any connection in group is dirty */ +}; +DEFINE_VECTOR_TYPE(group_vector, struct group *); +static group_vector groups = empty_vector; -/* Accept 'multi-conn-mode=mode' and 'multi-conn-track-dirty=level' */ +/* Accept 'multi-conn-mode=mode', 'multi-conn-track-dirty=level', and + * 'multi-conn-exportname=bool'. + */ static int multi_conn_config (nbdkit_next_config *next, void *nxdata, const char *key, const char *value) @@ -115,13 +126,24 @@ multi_conn_config (nbdkit_next_config *next, void *nxdata, } return 0; } + else if (strcmp (key, "multi-conn-exportname") == 0 || + strcmp (key, "multi-conn-export-name") == 0) { + int r; + + r = nbdkit_parse_bool (value); + if (r == -1) + return -1; + byname = r; + 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" + "multi-conn-track-dirty=<LEVEL> 'conn' (default), 'fast', or 'off'.\n" \ + "multi-conn-exportname=<BOOL> true to limit emulation by export name.\n" static int multi_conn_get_ready (nbdkit_next_get_ready *next, void *nxdata, @@ -148,6 +170,13 @@ multi_conn_open (nbdkit_next_open *next, void *nxdata, nbdkit_error ("calloc: %m"); return NULL; } + if (byname) { + h->name = strdup (exportname); + if (h->name == NULL) { + nbdkit_error ("strdup: %m"); + return NULL; + } + } return h; } @@ -156,6 +185,8 @@ multi_conn_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, void *handle, int readonly) { struct handle *h = handle; + struct group *g; + bool new_group = false; int r; h->next_ops = next_ops; @@ -177,7 +208,39 @@ multi_conn_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, } ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); - return conns_vector_append (&conns, h); + if (byname) { + g = NULL; + for (size_t i = 0; i < groups.size; i++) + if (strcmp (groups.ptr[i]->name, h->name) == 0) { + g = groups.ptr[i]; + break; + } + } + else + g = groups.size ? groups.ptr[0] : NULL; + + if (!g) { + g = calloc (1, sizeof *g); + if (g == NULL) { + nbdkit_error ("calloc: %m"); + return -1; + } + if (group_vector_append (&groups, g) == -1) + return -1; + g->name = h->name; + h->name = NULL; + new_group = true; + } + if (conns_vector_append (&g->conns, h) == -1) { + if (new_group) { + group_vector_remove (&groups, groups.size - 1); + free (g->name); + free (g); + } + return -1; + } + h->group = g; + return 0; } static int @@ -189,21 +252,36 @@ multi_conn_finalize (struct nbdkit_next_ops *next_ops, void *nxdata, ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); assert (h->next_ops == next_ops); assert (h->nxdata == nxdata); + assert (h->group); /* 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); + for (size_t i = 0; i < h->group->conns.size; i++) { + if (h->group->conns.ptr[i] == h) { + conns_vector_remove (&h->group->conns, i); break; } } + if (h->group->conns.size == 0) { + for (size_t i = 0; i < groups.size; i++) + if (groups.ptr[i] == h->group) { + group_vector_remove (&groups, i); + free (h->group->name); + free (h->group); + break; + } + } + h->group = NULL; return 0; } static void multi_conn_close (void *handle) { - free (handle); + struct handle *h = handle; + + assert (h->group == NULL); + free (h->name); + free (h); } static int @@ -259,7 +337,7 @@ mark_dirty (struct handle *h, bool is_write) /* fallthrough */ case FAST: if (is_write) - dirty = true; + h->group->dirty = true; break; case OFF: break; @@ -376,22 +454,24 @@ multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, struct handle *h = handle, *h2; size_t i; + assert (h->group); 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)) || + for (i = 0; i < h->group->conns.size; i++) { + h2 = h->group->conns.ptr[i]; + if (track == OFF || (h->group->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; + h->group->dirty = 0; } else { /* !EMULATE: Check if the image is clean, allowing us to skip a flush. */ - if (track != OFF && !dirty) + if (track != OFF && !h->group->dirty) return 0; /* Perform the flush, then update dirty tracking. */ if (next_ops->flush (nxdata, flags, err) == -1) @@ -400,15 +480,15 @@ multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, 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; + for (i = 0; i < h->group->conns.size; i++) + h->group->conns.ptr[i]->dirty = 0; + h->group->dirty = 0; } else h->dirty = 0; break; case FAST: - dirty = false; + h->group->dirty = false; break; case OFF: break; diff --git a/tests/test-multi-conn-name.sh b/tests/test-multi-conn-name.sh new file mode 100755 index 00000000..698d386e --- /dev/null +++ b/tests/test-multi-conn-name.sh @@ -0,0 +1,88 @@ +#!/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 effect of multi-conn-exportname config flag + +source ./functions.sh +set -e +set -x + +requires_plugin sh +requires_nbdsh_uri +requires nbdsh -c 'print(h.set_opt_mode)' +requires dd iflag=count_bytes </dev/null + +files="test-multi-conn-name.out" +rm -f $files +cleanup_fn rm -f $files + +fail=0 +export script=' +import os + +uri = os.environ["uri"] +h = {} +for name in ["a", "b"]: + for conn in [1, 2]: + key = "%s%d" % (name, conn) + h[key] = nbd.NBD() + h[key].set_opt_mode(True) + h[key].connect_uri(uri) + h[key].set_export_name(name) + h[key].opt_go() +h["a1"].pread(1, 0) +h["a2"].pwrite(b"A", 0) +h["b1"].pread(1, 0) +h["b2"].pwrite(b"B", 0, nbd.CMD_FLAG_FUA) +print(h["a1"].pread(1, 0)) +print(h["b1"].pread(1, 0)) +' + +# Without the knob we flush all exports +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --run 'export uri; nbdsh -c "$script"' > test-multi-conn-name.out || fail=1 +diff -u <(cat <<\EOF +b'A' +b'B' +EOF + ) test-multi-conn-name.out || fail=1 +# But with the knob, our flush is specific to the correct export +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + multi-conn-exportname=true \ + --run 'export uri; nbdsh -c "$script"' > test-multi-conn-name.out || fail=1 +diff -u <(cat <<\EOF +b'I' +b'B' +EOF + ) test-multi-conn-name.out || fail=1 + +exit $fail diff --git a/tests/test-multi-conn-plugin.sh b/tests/test-multi-conn-plugin.sh index 7262a348..5d627a07 100755 --- a/tests/test-multi-conn-plugin.sh +++ b/tests/test-multi-conn-plugin.sh @@ -39,30 +39,36 @@ # it ever sends overlapping writes without coordinating flushes and still # expects any particular write to occur last). +get_export() { + case $1 in + */*) export="$tmpdir/$(dirname $1)" conn=$(basename $1) ;; + *) export="$tmpdir" conn=$1 ;; + esac +} fill_cache() { - if test ! -f "$tmpdir/$1"; then - cp "$tmpdir/0" "$tmpdir/$1" + if test ! -f "$export/$conn"; then + cp "$export/0" "$export/$conn" fi } do_fua() { - case ,$4, in + case ,$3, in *,fua,*) if test -f "$tmpdir/strictfua"; then - dd of="$tmpdir/0" if="$tmpdir/$1" skip=$3 seek=$3 count=$2 \ + dd of="$export/0" if="$export/$conn" skip=$2 seek=$2 count=$1 \ conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes else - do_flush $1 + do_flush fi ;; esac } do_flush() { - if test -f "$tmpdir/$1-replay"; then + if test -f "$export/$conn-replay"; then while read cnt off; do - dd of="$tmpdir/0" if="$tmpdir/$1" skip=$off seek=$off count=$cnt \ + dd of="$export/0" if="$export/$conn" skip=$off seek=$off count=$cnt \ conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes - done < "$tmpdir/$1-replay" + done < "$export/$conn-replay" fi - rm -f "$tmpdir/$1" "$tmpdir/$1-replay" + rm -f "$export/$conn" "$export/$conn-replay" } case "$1" in config) @@ -91,30 +97,43 @@ case "$1" in ;; open) read i < "$tmpdir/counter" - echo $((i+1)) | tee "$tmpdir/counter" + i=$((i+1)) + echo $i > "$tmpdir/counter" + if test -z "$3"; then + echo $i + else + mkdir -p "$tmpdir/$3" || exit 1 + cp "$tmpdir/0" "$tmpdir/$3/0" + echo "$3/$i" + fi ;; pread) - fill_cache $2 - dd if="$tmpdir/$2" skip=$4 count=$3 iflag=count_bytes,skip_bytes + get_export $2 + fill_cache + dd if="$export/$conn" skip=$4 count=$3 iflag=count_bytes,skip_bytes ;; cache) - fill_cache $2 + get_export $2 + fill_cache ;; 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 + get_export $2 + fill_cache + dd of="$export/$conn" seek=$4 conv=notrunc oflag=seek_bytes + echo $3 $4 >> "$export/$conn-replay" + do_fua $3 $4 $5 ;; zero | trim) - fill_cache $2 - dd of="$tmpdir/$2" if="/dev/zero" count=$3 seek=$4 conv=notrunc\ + get_export $2 + fill_cache + dd of="$export/$conn" 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 + echo $3 $4 >> "$export/$conn-replay" + do_fua $3 $4 $5 ;; flush) - do_flush $2 + get_export $2 + do_flush ;; *) exit 2 -- 2.30.1
Richard W.M. Jones
2021-Mar-01 15:53 UTC
[Libguestfs] [nbdkit PATCH v2 4/6] filters: Track next handle alongside next backend
So ... this is all complicated ... And reverts some changes which I found simplified things. Stepping back here, is it the case that next-ops is really a hack, and what filters should actually get is some kind of connection object (let's call it that, even if it doesn't exactly correspond to "struct connection"), on which they can make general calls to the layer beneath. Then we allow filters to open and close new connection objects. How do filters get these connection objects? I would say that unlike next-ops, filters should not get a connection object with each callback. Instead in filter_open the filter should explicitly open a new connection, and in filter_close explicitly close it. Then filter_pread would make a pread() call on the connection object which the filter has saved in the filter's handle. My questions would be, assuming a design like this magically existed: (1) Does it solve the problem for multi-conn filter? (2) Does it solve the problem for background threads? Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/