[PATCH nbdkit v2 0/6] New ones plugin and evil filter
This addresses most of the issues from the review of v1 here: https://listman.redhat.com/archives/libguestfs/2023-May/031520.html * The filter now only handles 1e-12 <= P <= 1/8. Probabilities outside this range are treated as 0% or 100% respectively. The reasons are given in the code and man page, but basically are to do with stopping things from blowing up. The restriction is not essential - it might be fixed in future. I also added tests for small and large probabilities. * I did *not* change probability parsing or the "inf/nan" etc case or return the numerator and denominator separately. The parsing could be changed/fixed later. The API/ABI could not be changed, but I'm unconvinced that returning two numbers instead of one does anything except push the problem elsewhere. * A new commit is added to make <ispowerof2.h> log_2_bits work on 64 bit ints on all platforms. I tested this inside a 32 bit mock. * Add more debugging, and in particular print the seed. * Use Python to count bits instead of scary shell script. * When comparing subsets in the tests, make sure they contain a stuck bit. * Use '**' for exponentiation consistently. * Various other comments / typos / etc fixed and some improvements to the man page. * Retest. Now I must get back to doing real work! Rich.
In nbdkit-error-filter we need to parse parameters as probabilities. This is useful enough to add to nbdkit, since we will use it in another filter in future. --- docs/nbdkit-plugin.pod | 19 +++++++ plugins/python/nbdkit-python-plugin.pod | 6 ++ include/nbdkit-common.h | 2 + server/nbdkit.syms | 1 + server/public.c | 37 +++++++++++++ server/test-public.c | 73 +++++++++++++++++++++++++ plugins/ocaml/NBDKit.mli | 11 ++-- plugins/ocaml/NBDKit.ml | 2 + plugins/ocaml/bindings.c | 16 ++++++ plugins/python/modfunctions.c | 21 +++++++ tests/test-python-plugin.py | 12 ++++ tests/test_ocaml_plugin.ml | 1 + 12 files changed, 196 insertions(+), 5 deletions(-) diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 860c5cecb..e8d30a98e 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -1433,6 +1433,25 @@ C<str> can be a string containing a case-insensitive form of various common toggle values. The function returns 0 or 1 if the parse was successful. If there was an error, it returns C<-1>. +=head2 Parsing probabilities + +Use the C<nbdkit_parse_probability> utility function to parse +probabilities. Common formats understood include: C<"0.1">, C<"10%"> +or C<"1:10">, which all mean a probability of 1 in 10. + + int nbdkit_parse_probability (const char *what, const char *str, + double *ret); + +The C<what> parameter is printed in error messages to provide context. +The C<str> parameter is the probability string. + +On success the function returns C<0> and sets C<*ret>. B<Note> that +the probability returned may be outside the range S<[ 0.0..1.0 ]>, for +example if C<str == "200%">. If you want to clamp the result you must +check that yourself. + +On error, nbdkit_error is called and C<-1> is returned. + =head2 Reading passwords The C<nbdkit_read_password> utility function can be used to read diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod index e328cc2e0..0e55dcfcc 100644 --- a/plugins/python/nbdkit-python-plugin.pod +++ b/plugins/python/nbdkit-python-plugin.pod @@ -136,6 +136,12 @@ C<import errno>. Parse a string (such as "100M") into a size in bytes. Wraps the C<nbdkit_parse_size()> C function. +=head3 C<nbdkit.parse_probability(what, str)> + +Parse a string (such as "100%") into a probability, returning a +floating point number. Wraps the C<nbdkit_parse_probability()> C +function. + =head3 C<nbdkit.shutdown()> Request asynchronous server shutdown. diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h index e070245b8..b44e77323 100644 --- a/include/nbdkit-common.h +++ b/include/nbdkit-common.h @@ -105,6 +105,8 @@ NBDKIT_EXTERN_DECL (void, nbdkit_vdebug, NBDKIT_EXTERN_DECL (char *, nbdkit_absolute_path, (const char *path)); NBDKIT_EXTERN_DECL (int64_t, nbdkit_parse_size, (const char *str)); +NBDKIT_EXTERN_DECL (int, nbdkit_parse_probability, + (const char *what, const char *str, double *r)); NBDKIT_EXTERN_DECL (int, nbdkit_parse_bool, (const char *str)); NBDKIT_EXTERN_DECL (int, nbdkit_parse_int, (const char *what, const char *str, int *r)); diff --git a/server/nbdkit.syms b/server/nbdkit.syms index 353ea2a98..c6480e43e 100644 --- a/server/nbdkit.syms +++ b/server/nbdkit.syms @@ -67,6 +67,7 @@ nbdkit_parse_int64_t; nbdkit_parse_int8_t; nbdkit_parse_int; + nbdkit_parse_probability; nbdkit_parse_size; nbdkit_parse_uint16_t; nbdkit_parse_uint32_t; diff --git a/server/public.c b/server/public.c index 71ea6779d..fe2b48c7c 100644 --- a/server/public.c +++ b/server/public.c @@ -421,6 +421,43 @@ nbdkit_parse_size (const char *str) return size * scale; } +NBDKIT_DLL_PUBLIC int +nbdkit_parse_probability (const char *what, const char *str, + double *retp) +{ + double d, d2; + char c; + int n; + + if (sscanf (str, "%lg%[:/]%lg%n", &d, &c, &d2, &n) == 3 && + strcmp (&str[n], "") == 0) { /* N:M or N/M */ + if (d == 0 && d2 == 0) /* 0/0 is OK */ + ; + else if (d2 == 0) /* N/0 is bad */ + goto bad_parse; + else + d /= d2; + } + else if (sscanf (str, "%lg%n", &d, &n) == 1) { + if (strcmp (&str[n], "%") == 0) /* percentage */ + d /= 100.0; + else if (strcmp (&str[n], "") == 0) /* probability */ + ; + else + goto bad_parse; + } + else + goto bad_parse; + + if (retp) + *retp = d; + return 0; + + bad_parse: + nbdkit_error ("%s: could not parse '%s'", what, str); + return -1; +} + /* Parse a string as a boolean, or return -1 after reporting the error. */ NBDKIT_DLL_PUBLIC int diff --git a/server/test-public.c b/server/test-public.c index 676411290..0d84abdd2 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -200,6 +200,78 @@ test_nbdkit_parse_size (void) return pass; } +static bool +test_nbdkit_parse_probability (void) +{ + size_t i; + bool pass = true; + struct pair { + const char *str; + int result; + double expected; + } tests[] = { + /* Bogus strings */ + { "", -1 }, + { "garbage", -1 }, + { "0garbage", -1 }, + { "1X", -1 }, + { "1%%", -1 }, + { "1:", -1 }, + { "1:1:1", -1 }, + { "1:0", -1 }, /* format is valid but divide by zero is not allowed */ + { "1/", -1 }, + { "1/2/3", -1 }, + + /* Numbers. */ + { "0", 0, 0 }, + { "1", 0, 1 }, + { "2", 0, 2 }, /* values outside [0..1] range are allowed */ + { "0.1", 0, 0.1 }, + { "0.5", 0, 0.5 }, + { "0.9", 0, 0.9 }, + { "1.0000", 0, 1 }, + + /* Percentages. */ + { "0%", 0, 0 }, + { "50%", 0, 0.5 }, + { "100%", 0, 1 }, + { "90.25%", 0, 0.9025 }, + + /* N in M */ + { "1:1000", 0, 0.001 }, + { "1/1000", 0, 0.001 }, + { "2:99", 0, 2.0/99 }, + { "2/99", 0, 2.0/99 }, + { "0:1000000", 0, 0 }, + }; + + for (i = 0; i < ARRAY_SIZE (tests); i++) { + int r; + double d; + + error_flagged = false; + r = nbdkit_parse_probability ("test", tests[i].str, &d); + if (r != tests[i].result) { + fprintf (stderr, + "Wrong return value for %s, got %d, expected %d\n", + tests[i].str, r, tests[i].result); + pass = false; + } + if (r == 0 && d != tests[i].expected) { + fprintf (stderr, + "Wrong result for %s, got %g, expected %g\n", + tests[i].str, d, tests[i].expected); + pass = false; + } + if ((r == -1) != error_flagged) { + fprintf (stderr, "Wrong error message handling for %s\n", tests[i].str); + pass = false; + } + } + + return pass; +} + static bool test_nbdkit_parse_ints (void) { @@ -503,6 +575,7 @@ main (int argc, char *argv[]) { bool pass = true; pass &= test_nbdkit_parse_size (); + pass &= test_nbdkit_parse_probability (); pass &= test_nbdkit_parse_ints (); pass &= test_nbdkit_read_password (); /* nbdkit_absolute_path and nbdkit_nanosleep not unit-tested here, but diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli index 81447d07d..bc190f267 100644 --- a/plugins/ocaml/NBDKit.mli +++ b/plugins/ocaml/NBDKit.mli @@ -134,9 +134,9 @@ val register_plugin : into [EINVAL]. *) val set_error : Unix.error -> unit -(** Bindings for [nbdkit_parse_size], [nbdkit_parse_bool] and - [nbdkit_read_password]. See nbdkit-plugin(3) for information - about these functions. +(** Bindings for [nbdkit_parse_size], [nbdkit_parse_probability], + [nbdkit_parse_bool] and [nbdkit_read_password]. See + nbdkit-plugin(3) for information about these functions. On error these functions all raise [Invalid_argument]. The actual error is sent to the nbdkit error log and is not @@ -145,10 +145,11 @@ val set_error : Unix.error -> unit (* Note OCaml has functions already for parsing other integers, so * there is no need to bind them here. We only bind the functions * which have special abilities in nbdkit: [parse_size] can parse - * human sizes, [parse_bool] parses a range of nbdkit-specific - * boolean strings, and [read_password] suppresses echo. + * human sizes, [parse_probability] and [parse_bool] parses a range + * of nbdkit-specific strings, and [read_password] suppresses echo. *) val parse_size : string -> int64 +val parse_probability : string -> string -> float val parse_bool : string -> bool val read_password : string -> string diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml index e1cf28c94..2d1696917 100644 --- a/plugins/ocaml/NBDKit.ml +++ b/plugins/ocaml/NBDKit.ml @@ -160,6 +160,8 @@ let register_plugin ~name (* Bindings to nbdkit server functions. *) external set_error : Unix.error -> unit = "ocaml_nbdkit_set_error" [@@noalloc] external parse_size : string -> int64 = "ocaml_nbdkit_parse_size" +external parse_probability : string -> string -> float = + "ocaml_nbdkit_parse_probability" external parse_bool : string -> bool = "ocaml_nbdkit_parse_bool" external read_password : string -> string = "ocaml_nbdkit_read_password" external realpath : string -> string = "ocaml_nbdkit_realpath" diff --git a/plugins/ocaml/bindings.c b/plugins/ocaml/bindings.c index f2c9ca07d..4885feac5 100644 --- a/plugins/ocaml/bindings.c +++ b/plugins/ocaml/bindings.c @@ -74,6 +74,22 @@ ocaml_nbdkit_parse_size (value strv) CAMLreturn (rv); } +NBDKIT_DLL_PUBLIC value +ocaml_nbdkit_parse_probability (value whatv, value strv) +{ + CAMLparam2 (whatv, strv); + CAMLlocal1 (dv); + int r; + double d; + + r = nbdkit_parse_probability (String_val (whatv), String_val (strv), &d); + if (r == -1) + caml_invalid_argument ("nbdkit_parse_probability"); + dv = caml_copy_double (d); + + CAMLreturn (dv); +} + NBDKIT_DLL_PUBLIC value ocaml_nbdkit_parse_bool (value strv) { diff --git a/plugins/python/modfunctions.c b/plugins/python/modfunctions.c index 479707e78..74bfd6f27 100644 --- a/plugins/python/modfunctions.c +++ b/plugins/python/modfunctions.c @@ -122,11 +122,32 @@ parse_size (PyObject *self, PyObject *args) return PyLong_FromSize_t ((size_t)size); } +/* nbdkit.parse_probability */ +static PyObject * +parse_probability (PyObject *self, PyObject *args) +{ + const char *what, *str; + double d; + + if (!PyArg_ParseTuple (args, "ss:parse_probability", &what, &str)) + return NULL; + + if (nbdkit_parse_probability (what, str, &d) == -1) { + PyErr_SetString (PyExc_ValueError, + "Unable to parse string as probability"); + return NULL; + } + + return PyFloat_FromDouble (d); +} + static PyMethodDef NbdkitMethods[] = { { "debug", debug, METH_VARARGS, "Print a debug message" }, { "export_name", export_name, METH_NOARGS, "Return the optional export name negotiated with the client" }, + { "parse_probability", parse_probability, METH_VARARGS, + "Parse probability strings into floating point number" }, { "parse_size", parse_size, METH_VARARGS, "Parse human-readable size strings into bytes" }, { "set_error", set_error, METH_VARARGS, diff --git a/tests/test-python-plugin.py b/tests/test-python-plugin.py index c3232a112..7f3a2c2e4 100644 --- a/tests/test-python-plugin.py +++ b/tests/test-python-plugin.py @@ -55,8 +55,20 @@ class TestAPI(unittest.TestCase): with self.assertRaises(ValueError): nbdkit.parse_size('foo') + def test_parse_probability(self): + self.assertEqual(nbdkit.parse_probability('test', '1:10'), 0.1) + self.assertEqual(nbdkit.parse_probability('test', '100%'), 1) + self.assertEqual(nbdkit.parse_probability('test', '0'), 0) + + with self.assertRaises(TypeError): + nbdkit.parse_probability('test', 17) + + with self.assertRaises(ValueError): + nbdkit.parse_probability('test', 'bar') + TestAPI().test_parse_size() +TestAPI().test_parse_probability() def config(k, v): diff --git a/tests/test_ocaml_plugin.ml b/tests/test_ocaml_plugin.ml index 8132de8f8..bf998d361 100644 --- a/tests/test_ocaml_plugin.ml +++ b/tests/test_ocaml_plugin.ml @@ -39,6 +39,7 @@ let sparse = Bytes.make nr_sectors '\000' (* sparseness bitmap *) (* Test parse_* functions. *) let () = assert (NBDKit.parse_size "1M" = Int64.of_int (1024*1024)); + assert (NBDKit.parse_probability "test parse probability" "1:10" = 0.1); assert (NBDKit.parse_bool "true" = true); assert (NBDKit.parse_bool "0" = false) -- 2.39.2
On Wed, May 17, 2023 at 11:06:54AM +0100, Richard W.M. Jones wrote:
In nbdkit-error-filter we need to parse parameters as probabilities. This is useful enough to add to nbdkit, since we will use it in another filter in future. --- docs/nbdkit-plugin.pod | 19 +++++++ plugins/python/nbdkit-python-plugin.pod | 6 ++ include/nbdkit-common.h | 2 + server/nbdkit.syms | 1 + server/public.c | 37 +++++++++++++ server/test-public.c | 73 +++++++++++++++++++++++++ plugins/ocaml/NBDKit.mli | 11 ++-- plugins/ocaml/NBDKit.ml | 2 + plugins/ocaml/bindings.c | 16 ++++++ plugins/python/modfunctions.c | 21 +++++++ tests/test-python-plugin.py | 12 ++++ tests/test_ocaml_plugin.ml | 1 + 12 files changed, 196 insertions(+), 5 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 860c5cecb..e8d30a98e 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -1433,6 +1433,25 @@ C<str> can be a string containing a case-insensitive form of various common toggle values. The function returns 0 or 1 if the parse was successful. If there was an error, it returns C<-1>.
+=head2 Parsing probabilities + +Use the C<nbdkit_parse_probability> utility function to parse +probabilities. Common formats understood include: C<"0.1">, C<"10%"> +or C<"1:10">, which all mean a probability of 1 in 10. + + int nbdkit_parse_probability (const char *what, const char *str, + double *ret); + +The C<what> parameter is printed in error messages to provide context. +The C<str> parameter is the probability string. + +On success the function returns C<0> and sets C<*ret>. B<Note> that +the probability returned may be outside the range S<[ 0.0..1.0 ]>, for +example if C<str == "200%">. If you want to clamp the result you must +check that yourself.
Should we at least guarantee that the return value is non-negative (other than potentially -0.0 which compares equal to 0.0) and finite? I don't see how a negative or infinite probability will ever be useful (with apologies to Douglas Adams' infinite improbability drive).
+++ b/server/public.c @@ -421,6 +421,43 @@ nbdkit_parse_size (const char *str) return size * scale; }
+NBDKIT_DLL_PUBLIC int +nbdkit_parse_probability (const char *what, const char *str, + double *retp) +{ + double d, d2; + char c; + int n; + + if (sscanf (str, "%lg%[:/]%lg%n", &d, &c, &d2, &n) == 3 && + strcmp (&str[n], "") == 0) { /* N:M or N/M */ + if (d == 0 && d2 == 0) /* 0/0 is OK */
I'd write this 'if (d == 0.0 && d2 == 0.0)' to make it obvious that we know we are doing floating point comparisons here (semantics are the same either way, though, because of how integer 0 promotes under standard arithmetic promotion).
+ ; + else if (d2 == 0) /* N/0 is bad */ + goto bad_parse; + else + d /= d2; + } + else if (sscanf (str, "%lg%n", &d, &n) == 1) { + if (strcmp (&str[n], "%") == 0) /* percentage */ + d /= 100.0; + else if (strcmp (&str[n], "") == 0) /* probability */ + ; + else + goto bad_parse; + } + else + goto bad_parse;
Here's where it might be worth adding: if (d < 0.0 || !isfinite (d)) goto bad_parse; to filter out the cases where the user passed in "inf", "nan", or even some form of large/small that results in overflow. You caould also use 'signbit (d)' instead of 'd < 0.0' if you want to prevent -0.0 from causing surprises (while IEEE 754 and therefore the compiler treats '0.0 == -0.0' as true despite being different bit patterns, x/0.0 and x/-0.0 for finite x differ in behavior).
+ + if (retp) + *retp = d; + return 0; + + bad_parse: + nbdkit_error ("%s: could not parse '%s'", what, str);
Should this say "could not parse '%s' as a probability" to help the user search the documntation for what forms can be parsed?
+ return -1;
If you get here, *retp was unchanged. [1]
+} + /* Parse a string as a boolean, or return -1 after reporting the error. */ NBDKIT_DLL_PUBLIC int diff --git a/server/test-public.c b/server/test-public.c index 676411290..0d84abdd2 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -200,6 +200,78 @@ test_nbdkit_parse_size (void) return pass; }
+static bool +test_nbdkit_parse_probability (void) +{ + size_t i; + bool pass = true; + struct pair { + const char *str; + int result; + double expected; + } tests[] = { + /* Bogus strings */ + { "", -1 }, + { "garbage", -1 }, + { "0garbage", -1 }, + { "1X", -1 }, + { "1%%", -1 }, + { "1:", -1 }, + { "1:1:1", -1 }, + { "1:0", -1 }, /* format is valid but divide by zero is not allowed */ + { "1/", -1 }, + { "1/2/3", -1 },
If we add my proposed check above for filtering out negatives and non-finite values, we could also test that things like "-1", "inf", "nan", "1e200/1e-200" are rejected. Likewise worth adding a test for "-0" (whether you decide to permit or reject it).
+ + /* Numbers. */ + { "0", 0, 0 }, + { "1", 0, 1 }, + { "2", 0, 2 }, /* values outside [0..1] range are allowed */ + { "0.1", 0, 0.1 }, + { "0.5", 0, 0.5 }, + { "0.9", 0, 0.9 }, + { "1.0000", 0, 1 }, + + /* Percentages. */ + { "0%", 0, 0 }, + { "50%", 0, 0.5 }, + { "100%", 0, 1 }, + { "90.25%", 0, 0.9025 }, + + /* N in M */ + { "1:1000", 0, 0.001 }, + { "1/1000", 0, 0.001 }, + { "2:99", 0, 2.0/99 }, + { "2/99", 0, 2.0/99 }, + { "0:1000000", 0, 0 },
Since you document in patch 5 that we have a default percentage of "1e-8", it is worth testing that as an explicitly supported string.
+ }; + + for (i = 0; i < ARRAY_SIZE (tests); i++) { + int r; + double d;
Uninitialized...
+ + error_flagged = false; + r = nbdkit_parse_probability ("test", tests[i].str, &d);
...and per [1] above, there are code paths where d is not assigned...
+ if (r != tests[i].result) { + fprintf (stderr, + "Wrong return value for %s, got %d, expected %d\n", + tests[i].str, r, tests[i].result); + pass = false; + } + if (r == 0 && d != tests[i].expected) { + fprintf (stderr, + "Wrong result for %s, got %g, expected %g\n", + tests[i].str, d, tests[i].expected);
...so this can end up printing garbage. You may want to consider having nbdkit_parse_probability assign into d on all code paths, not just success.
+ pass = false; + } + if ((r == -1) != error_flagged) { + fprintf (stderr, "Wrong error message handling for %s\n", tests[i].str); + pass = false; + } + } + + return pass; +} + static bool test_nbdkit_parse_ints (void) { @@ -503,6 +575,7 @@ main (int argc, char *argv[]) { bool pass = true; pass &= test_nbdkit_parse_size (); + pass &= test_nbdkit_parse_probability (); pass &= test_nbdkit_parse_ints (); pass &= test_nbdkit_read_password (); /* nbdkit_absolute_path and nbdkit_nanosleep not unit-tested here, but
+++ b/plugins/ocaml/NBDKit.ml @@ -160,6 +160,8 @@ let register_plugin ~name (* Bindings to nbdkit server functions. *) external set_error : Unix.error -> unit = "ocaml_nbdkit_set_error" [@@noalloc] external parse_size : string -> int64 = "ocaml_nbdkit_parse_size" +external parse_probability : string -> string -> float = + "ocaml_nbdkit_parse_probability"
Is OCaml 'float' required to be any specific floating point (such as IEEE 754 binary64), or is it some nebulous hardware-specific floating point (possibly 32-bit instead of 64-bit, or even permitting VAX instead of IEEE)? But how OCaml maps floating point is not a show-stopper to this patch, since we already state OCaml bindings don't have ABI stability like C bindings. (I understand why modern languages have picked 'float' to mean floating-point while still favoring the IEEE 754 binary64 representations, where C is the odd one out that picked 'float' as binary32 and is stuck with the type name 'double' which has no resemblance to floating-point but rather to its size difference.) [Side note: if you really want a trip, read the 2023 SIGBOVIK article on "GradIEEEnt half decent" about 16-bit floating point values being exploited for their non-linear rounding properties as a way to create non-monotonic functions that can in turn form the basis of a Turing complete system capable of running a 36-second solution of Mario level 1-1 in 19k minutes of wall time using only half-precision floating-point operations... https://sigbovik.org/2023/, http://tom7.org/grad/murphy2023grad.pdf]
+++ b/plugins/python/modfunctions.c @@ -122,11 +122,32 @@ parse_size (PyObject *self, PyObject *args) return PyLong_FromSize_t ((size_t)size); }
+/* nbdkit.parse_probability */ +static PyObject * +parse_probability (PyObject *self, PyObject *args) +{ + const char *what, *str; + double d; + + if (!PyArg_ParseTuple (args, "ss:parse_probability", &what, &str)) + return NULL; + + if (nbdkit_parse_probability (what, str, &d) == -1) { + PyErr_SetString (PyExc_ValueError, + "Unable to parse string as probability"); + return NULL; + } + + return PyFloat_FromDouble (d);
Another language binding that does not directly guarantee that 'Float' is an IEEE 64-bit value, but where we are probably safe: https://stackoverflow.com/questions/70184494/on-what-systems-does-python-not... -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, May 17, 2023 at 04:05:23PM -0500, Eric Blake wrote:
On Wed, May 17, 2023 at 11:06:54AM +0100, Richard W.M. Jones wrote:
In nbdkit-error-filter we need to parse parameters as probabilities. This is useful enough to add to nbdkit, since we will use it in another filter in future. --- docs/nbdkit-plugin.pod | 19 +++++++ plugins/python/nbdkit-python-plugin.pod | 6 ++ include/nbdkit-common.h | 2 + server/nbdkit.syms | 1 + server/public.c | 37 +++++++++++++ server/test-public.c | 73 +++++++++++++++++++++++++ plugins/ocaml/NBDKit.mli | 11 ++-- plugins/ocaml/NBDKit.ml | 2 + plugins/ocaml/bindings.c | 16 ++++++ plugins/python/modfunctions.c | 21 +++++++ tests/test-python-plugin.py | 12 ++++ tests/test_ocaml_plugin.ml | 1 + 12 files changed, 196 insertions(+), 5 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 860c5cecb..e8d30a98e 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -1433,6 +1433,25 @@ C<str> can be a string containing a case-insensitive form of various common toggle values. The function returns 0 or 1 if the parse was successful. If there was an error, it returns C<-1>.
+=head2 Parsing probabilities + +Use the C<nbdkit_parse_probability> utility function to parse +probabilities. Common formats understood include: C<"0.1">, C<"10%"> +or C<"1:10">, which all mean a probability of 1 in 10. + + int nbdkit_parse_probability (const char *what, const char *str, + double *ret); + +The C<what> parameter is printed in error messages to provide context. +The C<str> parameter is the probability string. + +On success the function returns C<0> and sets C<*ret>. B<Note> that +the probability returned may be outside the range S<[ 0.0..1.0 ]>, for +example if C<str == "200%">. If you want to clamp the result you must +check that yourself.
Should we at least guarantee that the return value is non-negative (other than potentially -0.0 which compares equal to 0.0) and finite? I don't see how a negative or infinite probability will ever be useful (with apologies to Douglas Adams' infinite improbability drive).
Looking at this in the cold light of day I think my confusion is around probabilities versus percentages/fractions. Actual probabilities can't be outside the range [0..1]. But it's usual to have a percentage above 100%, eg. you could have a base rate and then express 200% (2 times) this rate. We can use this function to parse percentages as well as probabilties, eg in an imaginary rate filter: bandwidth=10000 burst=200% or: bandwidth=10000 burst=2:1 But I don't think negative percentages make sense. So yes we can limit this to returning only numbers >= 0 (and also non NaN, not infinite).
+++ b/server/public.c @@ -421,6 +421,43 @@ nbdkit_parse_size (const char *str) return size * scale; }
+NBDKIT_DLL_PUBLIC int +nbdkit_parse_probability (const char *what, const char *str, + double *retp) +{ + double d, d2; + char c; + int n; + + if (sscanf (str, "%lg%[:/]%lg%n", &d, &c, &d2, &n) == 3 && + strcmp (&str[n], "") == 0) { /* N:M or N/M */ + if (d == 0 && d2 == 0) /* 0/0 is OK */
I'd write this 'if (d == 0.0 && d2 == 0.0)' to make it obvious that we know we are doing floating point comparisons here (semantics are the same either way, though, because of how integer 0 promotes under standard arithmetic promotion).
OK.
+ ; + else if (d2 == 0) /* N/0 is bad */ + goto bad_parse; + else + d /= d2; + } + else if (sscanf (str, "%lg%n", &d, &n) == 1) { + if (strcmp (&str[n], "%") == 0) /* percentage */ + d /= 100.0; + else if (strcmp (&str[n], "") == 0) /* probability */ + ; + else + goto bad_parse; + } + else + goto bad_parse;
Here's where it might be worth adding:
if (d < 0.0 || !isfinite (d)) goto bad_parse;
to filter out the cases where the user passed in "inf", "nan", or even some form of large/small that results in overflow.
You caould also use 'signbit (d)' instead of 'd < 0.0' if you want to prevent -0.0 from causing surprises (while IEEE 754 and therefore the compiler treats '0.0 == -0.0' as true despite being different bit patterns, x/0.0 and x/-0.0 for finite x differ in behavior).
OK, but added to nbdkit_parse_probability itself.
+ + if (retp) + *retp = d; + return 0; + + bad_parse: + nbdkit_error ("%s: could not parse '%s'", what, str);
Should this say "could not parse '%s' as a probability" to help the user search the documntation for what forms can be parsed?
Yes, it would also be good to mention the function name.
+ return -1;
If you get here, *retp was unchanged. [1]
+} + /* Parse a string as a boolean, or return -1 after reporting the error. */ NBDKIT_DLL_PUBLIC int diff --git a/server/test-public.c b/server/test-public.c index 676411290..0d84abdd2 100644 --- a/server/test-public.c +++ b/server/test-public.c @@ -200,6 +200,78 @@ test_nbdkit_parse_size (void) return pass; }
+static bool +test_nbdkit_parse_probability (void) +{ + size_t i; + bool pass = true; + struct pair { + const char *str; + int result; + double expected; + } tests[] = { + /* Bogus strings */ + { "", -1 }, + { "garbage", -1 }, + { "0garbage", -1 }, + { "1X", -1 }, + { "1%%", -1 }, + { "1:", -1 }, + { "1:1:1", -1 }, + { "1:0", -1 }, /* format is valid but divide by zero is not allowed */ + { "1/", -1 }, + { "1/2/3", -1 },
If we add my proposed check above for filtering out negatives and non-finite values, we could also test that things like "-1", "inf", "nan", "1e200/1e-200" are rejected. Likewise worth adding a test for "-0" (whether you decide to permit or reject it).
OK.
+ + /* Numbers. */ + { "0", 0, 0 }, + { "1", 0, 1 }, + { "2", 0, 2 }, /* values outside [0..1] range are allowed */ + { "0.1", 0, 0.1 }, + { "0.5", 0, 0.5 }, + { "0.9", 0, 0.9 }, + { "1.0000", 0, 1 }, + + /* Percentages. */ + { "0%", 0, 0 }, + { "50%", 0, 0.5 }, + { "100%", 0, 1 }, + { "90.25%", 0, 0.9025 }, + + /* N in M */ + { "1:1000", 0, 0.001 }, + { "1/1000", 0, 0.001 }, + { "2:99", 0, 2.0/99 }, + { "2/99", 0, 2.0/99 }, + { "0:1000000", 0, 0 },
Since you document in patch 5 that we have a default percentage of "1e-8", it is worth testing that as an explicitly supported string.
OK.
+ }; + + for (i = 0; i < ARRAY_SIZE (tests); i++) { + int r; + double d;
Uninitialized...
+ + error_flagged = false; + r = nbdkit_parse_probability ("test", tests[i].str, &d);
...and per [1] above, there are code paths where d is not assigned...
+ if (r != tests[i].result) { + fprintf (stderr, + "Wrong return value for %s, got %d, expected %d\n", + tests[i].str, r, tests[i].result); + pass = false; + } + if (r == 0 && d != tests[i].expected) { + fprintf (stderr, + "Wrong result for %s, got %g, expected %g\n", + tests[i].str, d, tests[i].expected);
...so this can end up printing garbage. You may want to consider having nbdkit_parse_probability assign into d on all code paths, not just success.
I don't think I understand this. Surely if r == 0 then d has been assigned, and if r == -1 we don't print d?
+ pass = false; + } + if ((r == -1) != error_flagged) { + fprintf (stderr, "Wrong error message handling for %s\n", tests[i].str); + pass = false; + } + } + + return pass; +} + static bool test_nbdkit_parse_ints (void) { @@ -503,6 +575,7 @@ main (int argc, char *argv[]) { bool pass = true; pass &= test_nbdkit_parse_size (); + pass &= test_nbdkit_parse_probability (); pass &= test_nbdkit_parse_ints (); pass &= test_nbdkit_read_password (); /* nbdkit_absolute_path and nbdkit_nanosleep not unit-tested here, but
+++ b/plugins/ocaml/NBDKit.ml @@ -160,6 +160,8 @@ let register_plugin ~name (* Bindings to nbdkit server functions. *) external set_error : Unix.error -> unit = "ocaml_nbdkit_set_error" [@@noalloc] external parse_size : string -> int64 = "ocaml_nbdkit_parse_size" +external parse_probability : string -> string -> float = + "ocaml_nbdkit_parse_probability"
Is OCaml 'float' required to be any specific floating point (such as IEEE 754 binary64), or is it some nebulous hardware-specific floating point (possibly 32-bit instead of 64-bit, or even permitting VAX instead of IEEE)? But how OCaml maps floating point is not a show-stopper to this patch, since we already state OCaml bindings don't have ABI stability like C bindings. (I understand why modern languages have picked 'float' to mean floating-point while still favoring the IEEE 754 binary64 representations, where C is the odd one out that picked 'float' as binary32 and is stuck with the type name 'double' which has no resemblance to floating-point but rather to its size difference.)
I believe it's the same as a C double. There's no standard for OCaml so it's just whatever the current implementation does.
[Side note: if you really want a trip, read the 2023 SIGBOVIK article on "GradIEEEnt half decent" about 16-bit floating point values being exploited for their non-linear rounding properties as a way to create non-monotonic functions that can in turn form the basis of a Turing complete system capable of running a 36-second solution of Mario level 1-1 in 19k minutes of wall time using only half-precision floating-point operations... https://sigbovik.org/2023/, http://tom7.org/grad/murphy2023grad.pdf]
Will do :-/
+++ b/plugins/python/modfunctions.c @@ -122,11 +122,32 @@ parse_size (PyObject *self, PyObject *args) return PyLong_FromSize_t ((size_t)size); }
+/* nbdkit.parse_probability */ +static PyObject * +parse_probability (PyObject *self, PyObject *args) +{ + const char *what, *str; + double d; + + if (!PyArg_ParseTuple (args, "ss:parse_probability", &what, &str)) + return NULL; + + if (nbdkit_parse_probability (what, str, &d) == -1) { + PyErr_SetString (PyExc_ValueError, + "Unable to parse string as probability"); + return NULL; + } + + return PyFloat_FromDouble (d);
Another language binding that does not directly guarantee that 'Float' is an IEEE 64-bit value, but where we are probably safe: https://stackoverflow.com/questions/70184494/on-what-systems-does-python-not...
Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
On 5/17/23 23:05, Eric Blake wrote:
[Side note: if you really want a trip, read the 2023 SIGBOVIK article on "GradIEEEnt half decent" about 16-bit floating point values being exploited for their non-linear rounding properties as a way to create non-monotonic functions that can in turn form the basis of a Turing complete system capable of running a 36-second solution of Mario level 1-1 in 19k minutes of wall time using only half-precision floating-point operations... https://sigbovik.org/2023/, http://tom7.org/grad/murphy2023grad.pdf]
I've read the first few pages of this paper. It's amazing how capable and dedicated the author is! (And I love his humor!) Anyway: one of the footnotes says, There is seldom reason to change the rounding mode, and since it is a stateful act, you’re asking for it if you do. But the round-to- negative-infinity and round-to-positive-infinity modes are are useful for interval arithmetic, which is arguably the only truly reasonable way to use floating point. What you do is represent numbers as intervals (low and high endpoints) that contain the true value, and then perform each calculation on both endpoints. For computations on the low endpoint, you round down, and symmetrically for the high endpoint. This way, the true value is always within the interval, and you also know how much inaccuracy you have accumulated! This echoes my earlier encounters with "interval arithmetic", and only strengthens my aversion towards floating point in the present context. Please proceed with collaborating on this series, I need to withdraw from it. Laszlo
This replaces the existing ad hoc parsing with the new public function. --- filters/error/nbdkit-error-filter.pod | 12 +++++++----- filters/error/error.c | 14 +------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/filters/error/nbdkit-error-filter.pod b/filters/error/nbdkit-error-filter.pod index bedd97924..49f21d10a 100644 --- a/filters/error/nbdkit-error-filter.pod +++ b/filters/error/nbdkit-error-filter.pod @@ -6,7 +6,7 @@ nbdkit-error-filter - inject errors for testing clients nbdkit --filter=error PLUGIN [error=EPERM|EIO|ENOMEM|EINVAL|ENOSPC|ESHUTDOWN] - [error-rate=10%|0.1] + [error-rate=10%|0.1|1:10] [error-file=/tmp/inject] [error-pread=...] [error-pread-rate=...] [error-pread-file=...] [error-pwrite=...] [error-pwrite-rate=...] [error-pwrite-file=...] @@ -66,11 +66,13 @@ This parameter is optional and the default is C<EIO> =item B<error-rate=>0..1 +=item B<error-rate=>NB<:>M + The rate of injected errors per NBD request. This can be expressed as -either a percentage between C<0%> and C<100%> or as a probability -between C<0> and C<1>. If C<0%> or C<0> is used then no errors are -ever injected, and if C<100%> or C<1> is used then all requests return -errors. +a percentage between C<0%> and C<100%>, or as a probability between +C<0> and C<1>, or as a ratio like C<1:10>. If C<0%> or C<0> is used +then no errors are ever injected, and if C<100%> or C<1> is used then +all requests return errors. This parameter is optional and the default is C<0%>. B<Unless you set this, the filter will do nothing.> diff --git a/filters/error/error.c b/filters/error/error.c index ac4da8c16..08e19e3bd 100644 --- a/filters/error/error.c +++ b/filters/error/error.c @@ -130,21 +130,9 @@ static int parse_error_rate (const char *key, const char *value, double *retp) { double d; - int n; - if (sscanf (value, "%lg%n", &d, &n) == 1) { - if (strcmp (&value[n], "%") == 0) /* percentage */ - d /= 100.0; - else if (strcmp (&value[n], "") == 0) /* probability */ - ; - else - goto bad_parse; - } - else { - bad_parse: - nbdkit_error ("%s: could not parse rate '%s'", key, value); + if (nbdkit_parse_probability (key, value, &d) == -1) return -1; - } if (d < 0 || d > 1) { nbdkit_error ("%s: rate out of range: '%s' parsed as %g", key, value, d); return -1; -- 2.39.2
On Wed, May 17, 2023 at 11:06:55AM +0100, Richard W.M. Jones wrote:
This replaces the existing ad hoc parsing with the new public function. --- filters/error/nbdkit-error-filter.pod | 12 +++++++----- filters/error/error.c | 14 +------------- 2 files changed, 8 insertions(+), 18 deletions(-)
+ if (nbdkit_parse_probability (key, value, &d) == -1) return -1; - } if (d < 0 || d > 1) {
If you change nbdkit_parse_probability() based on my comments to patch 1 to guarantee a non-negative value in d on success, then then left half of this conditional would now be dead code. Otherwise, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
It takes a 64 bit integer and finds the next power of 2, eg. next_power_of_2 (510) => 512 (2^9) Taken from https://jameshfisher.com/2018/03/30/round-up-power-2/ with some fixes. Thanks: Eric Blake --- common/include/ispowerof2.h | 9 +++++++++ common/include/test-ispowerof2.c | 15 +++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h index f067caf07..a4cb52de3 100644 --- a/common/include/ispowerof2.h +++ b/common/include/ispowerof2.h @@ -59,4 +59,13 @@ log_2_bits (unsigned long v) return SIZEOF_LONG*8 - __builtin_clzl (v) - 1; } +/* Round up to next power of 2. + * https://jameshfisher.com/2018/03/30/round-up-power-2/ + */ +static inline uint64_t +next_power_of_2 (uint64_t x) +{ + return x <= 1 ? 1 : UINT64_C(1) << (64 - __builtin_clzll (x-1)); +} + #endif /* NBDKIT_ISPOWEROF2_H */ diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c index 9620192f0..1221ac09c 100644 --- a/common/include/test-ispowerof2.c +++ b/common/include/test-ispowerof2.c @@ -68,5 +68,20 @@ main (void) assert (log_2_bits (0x8000000000000000) == 63); #endif + /* Test next power of 2. */ + assert (next_power_of_2 (0) == 1); + assert (next_power_of_2 (1) == 1); + assert (next_power_of_2 (3) == 4); + assert (next_power_of_2 (8) == 8); + assert (next_power_of_2 (9) == 16); + assert (next_power_of_2 (0xffff) == 0x10000); + assert (next_power_of_2 (0x10000) == 0x10000); + assert (next_power_of_2 (UINT64_C ( 0xffffffff)) == 0x100000000); + assert (next_power_of_2 (UINT64_C (0x100000000)) == 0x100000000); + assert (next_power_of_2 (UINT64_C (0x200000001)) == 0x400000000); + assert (next_power_of_2 (UINT64_C (0x6ffffffff)) == 0x800000000); + assert (next_power_of_2 (UINT64_C (0x700000001)) == 0x800000000); + assert (next_power_of_2 (UINT64_C (0x800000000)) == 0x800000000); + exit (EXIT_SUCCESS); } -- 2.39.2
On Wed, May 17, 2023 at 11:06:56AM +0100, Richard W.M. Jones wrote:
It takes a 64 bit integer and finds the next power of 2, eg. next_power_of_2 (510) => 512 (2^9)
Taken from https://jameshfisher.com/2018/03/30/round-up-power-2/ with some fixes.
Thanks: Eric Blake --- common/include/ispowerof2.h | 9 +++++++++ common/include/test-ispowerof2.c | 15 +++++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h index f067caf07..a4cb52de3 100644 --- a/common/include/ispowerof2.h +++ b/common/include/ispowerof2.h @@ -59,4 +59,13 @@ log_2_bits (unsigned long v) return SIZEOF_LONG*8 - __builtin_clzl (v) - 1; }
+/* Round up to next power of 2. + * https://jameshfisher.com/2018/03/30/round-up-power-2/ + */ +static inline uint64_t +next_power_of_2 (uint64_t x) +{ + return x <= 1 ? 1 : UINT64_C(1) << (64 - __builtin_clzll (x-1)); +}
Still mishandles anything >= INT64_MAX+1ULL. __builtin_clzll(0xffffffffffffffffULL - 1) gives 0, but UINT64_C(1) << (64-0) is undefined behavior. We've already declared elsewhere that the maximum size that nbdkit will handle is 2^63-1 (ie. INT64_MAX), not 2^64-1 (UINT64_MAX); this is because off_t is a signed value, and it gets really hard to work with anything larger than what off_t can hold. But it's still probably worth documenting what we want to do for out-of-range values, rather than blindly assuming that plugins will only use this function on values that are already appropriate as sizes. Maybe adjust the signature to: static inline uint64_t next_power_of_2 (int64_t x) where all negative input values map to (uint64_t)(-1) as an out-of-range indicator?
+ #endif /* NBDKIT_ISPOWEROF2_H */ diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c index 9620192f0..1221ac09c 100644 --- a/common/include/test-ispowerof2.c +++ b/common/include/test-ispowerof2.c @@ -68,5 +68,20 @@ main (void) assert (log_2_bits (0x8000000000000000) == 63); #endif
+ /* Test next power of 2. */ + assert (next_power_of_2 (0) == 1);
Do we want this to be 1 (your choice of 'x <= 1 ? 1 : ...') or 0 (my suggestion of 'x <= 1 ? x : ...')?
+ assert (next_power_of_2 (1) == 1); + assert (next_power_of_2 (3) == 4); + assert (next_power_of_2 (8) == 8); + assert (next_power_of_2 (9) == 16); + assert (next_power_of_2 (0xffff) == 0x10000); + assert (next_power_of_2 (0x10000) == 0x10000); + assert (next_power_of_2 (UINT64_C ( 0xffffffff)) == 0x100000000); + assert (next_power_of_2 (UINT64_C (0x100000000)) == 0x100000000); + assert (next_power_of_2 (UINT64_C (0x200000001)) == 0x400000000); + assert (next_power_of_2 (UINT64_C (0x6ffffffff)) == 0x800000000); + assert (next_power_of_2 (UINT64_C (0x700000001)) == 0x800000000); + assert (next_power_of_2 (UINT64_C (0x800000000)) == 0x800000000);
While this tests that you handle more than 32 bits, it still doesn't test what happens when the most-significant bit is set (there's the special case of 0x8000000000000000ULL returning itself; all other values goes back to my question of whether we should have some sentinel for out-of-range). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Previously it only worked for 64 bit ints on 64 bit platforms, but we can easily adjust the function to work on any platform. --- common/include/ispowerof2.h | 4 ++-- common/include/test-ispowerof2.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h index a4cb52de3..8f56c8dbf 100644 --- a/common/include/ispowerof2.h +++ b/common/include/ispowerof2.h @@ -54,9 +54,9 @@ is_power_of_2 (unsigned long v) * __builtin_clzl is available in GCC and clang. */ static inline int -log_2_bits (unsigned long v) +log_2_bits (uint64_t v) { - return SIZEOF_LONG*8 - __builtin_clzl (v) - 1; + return 64 - __builtin_clzll (v) - 1; } /* Round up to next power of 2. diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c index 1221ac09c..09d248889 100644 --- a/common/include/test-ispowerof2.c +++ b/common/include/test-ispowerof2.c @@ -63,10 +63,8 @@ main (void) assert (log_2_bits (512) == 9); assert (log_2_bits (4096) == 12); assert (log_2_bits (0x80000000) == 31); -#if SIZEOF_LONG == 8 - assert (log_2_bits (0x100000000) == 32); - assert (log_2_bits (0x8000000000000000) == 63); -#endif + assert (log_2_bits (UINT64_C (0x100000000)) == 32); + assert (log_2_bits (UINT64_C (0x8000000000000000)) == 63); /* Test next power of 2. */ assert (next_power_of_2 (0) == 1); -- 2.39.2
On Wed, May 17, 2023 at 11:06:57AM +0100, Richard W.M. Jones wrote:
Previously it only worked for 64 bit ints on 64 bit platforms, but we can easily adjust the function to work on any platform. --- common/include/ispowerof2.h | 4 ++-- common/include/test-ispowerof2.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/common/include/ispowerof2.h b/common/include/ispowerof2.h index a4cb52de3..8f56c8dbf 100644 --- a/common/include/ispowerof2.h +++ b/common/include/ispowerof2.h @@ -54,9 +54,9 @@ is_power_of_2 (unsigned long v) * __builtin_clzl is available in GCC and clang. */ static inline int -log_2_bits (unsigned long v) +log_2_bits (uint64_t v) { - return SIZEOF_LONG*8 - __builtin_clzl (v) - 1; + return 64 - __builtin_clzll (v) - 1;
We already documented that this is undefined for v==0, and your change does not alter that. What we don't document is whether this is well-defined for a non-power-of-2...
}
/* Round up to next power of 2. diff --git a/common/include/test-ispowerof2.c b/common/include/test-ispowerof2.c index 1221ac09c..09d248889 100644 --- a/common/include/test-ispowerof2.c +++ b/common/include/test-ispowerof2.c @@ -63,10 +63,8 @@ main (void) assert (log_2_bits (512) == 9); assert (log_2_bits (4096) == 12); assert (log_2_bits (0x80000000) == 31); -#if SIZEOF_LONG == 8 - assert (log_2_bits (0x100000000) == 32); - assert (log_2_bits (0x8000000000000000) == 63); -#endif + assert (log_2_bits (UINT64_C (0x100000000)) == 32); + assert (log_2_bits (UINT64_C (0x8000000000000000)) == 63);
...we only test it on values where is_power_of_2() returns true. We should probably take this opportunity to either document that it is not just 0, but all non-power-of-2 values that have undefined behavior; or else to test some values with more than one bit set. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, May 17, 2023 at 11:06:57AM +0100, Richard W.M. Jones wrote:
Previously it only worked for 64 bit ints on 64 bit platforms, but we can easily adjust the function to work on any platform. --- common/include/ispowerof2.h | 4 ++-- common/include/test-ispowerof2.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-)
Should we also fix is_power_of_2() to work on 64-bit values on all platforms? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, May 17, 2023 at 04:36:32PM -0500, Eric Blake wrote:
On Wed, May 17, 2023 at 11:06:57AM +0100, Richard W.M. Jones wrote:
Previously it only worked for 64 bit ints on 64 bit platforms, but we can easily adjust the function to work on any platform. --- common/include/ispowerof2.h | 4 ++-- common/include/test-ispowerof2.c | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-)
Should we also fix is_power_of_2() to work on 64-bit values on all platforms?
Possibly, but I don't need that for this patch. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Returns a fully allocated disk containing all 0xff (all ones), or another byte of your choice. --- plugins/data/nbdkit-data-plugin.pod | 1 + plugins/full/nbdkit-full-plugin.pod | 1 + plugins/null/nbdkit-null-plugin.pod | 1 + plugins/ones/nbdkit-ones-plugin.pod | 77 +++++++ plugins/pattern/nbdkit-pattern-plugin.pod | 4 +- plugins/random/nbdkit-random-plugin.pod | 1 + .../nbdkit-sparse-random-plugin.pod | 1 + plugins/zero/nbdkit-zero-plugin.pod | 1 + configure.ac | 2 + plugins/ones/Makefile.am | 70 +++++++ tests/Makefile.am | 4 + plugins/ones/ones.c | 191 ++++++++++++++++++ tests/test-ones.sh | 53 +++++ 13 files changed, 406 insertions(+), 1 deletion(-) diff --git a/plugins/data/nbdkit-data-plugin.pod b/plugins/data/nbdkit-data-plugin.pod index cec1af7f7..46daa859e 100644 --- a/plugins/data/nbdkit-data-plugin.pod +++ b/plugins/data/nbdkit-data-plugin.pod @@ -404,6 +404,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-info-plugin(1)>, L<nbdkit-memory-plugin(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-partitioning-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-random-plugin(1)>, diff --git a/plugins/full/nbdkit-full-plugin.pod b/plugins/full/nbdkit-full-plugin.pod index 5664aa2da..18c86a327 100644 --- a/plugins/full/nbdkit-full-plugin.pod +++ b/plugins/full/nbdkit-full-plugin.pod @@ -65,6 +65,7 @@ L<nbdkit(1)>, L<nbdkit-plugin(3)>, L<nbdkit-error-filter(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-random-plugin(1)>. L<nbdkit-sparse-random-plugin(1)>, diff --git a/plugins/null/nbdkit-null-plugin.pod b/plugins/null/nbdkit-null-plugin.pod index 27fe68746..c71dc2c6f 100644 --- a/plugins/null/nbdkit-null-plugin.pod +++ b/plugins/null/nbdkit-null-plugin.pod @@ -55,6 +55,7 @@ C<nbdkit-null-plugin> first appeared in nbdkit 1.2. L<nbdkit(1)>, L<nbdkit-plugin(3)>, L<nbdkit-full-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-random-plugin(1)>. L<nbdkit-sparse-random-plugin(1)>, diff --git a/plugins/ones/nbdkit-ones-plugin.pod b/plugins/ones/nbdkit-ones-plugin.pod new file mode 100644 index 000000000..50ccfe272 --- /dev/null +++ b/plugins/ones/nbdkit-ones-plugin.pod @@ -0,0 +1,77 @@ +=head1 NAME + +nbdkit-ones-plugin - nbdkit plugin filled with repeated 0xff or other bytes + +=head1 SYNOPSIS + + nbdkit ones [[size=]SIZE] [byte=N] + +=head1 DESCRIPTION + +C<nbdkit-ones-plugin> is a plugin for L<nbdkit(1)>. Any read returns +a repeating pattern of C<0xff> (all ones) bytes, or another repeating +byte if you use the C<byte> parameter. Everything written to the +virtual device is discarded. + +Most NBD consumers will be very confused by this device. It is mainly +useful for testing. L<nbdkit-data-plugin(1)> can be used to create a +modifiable RAM disk initialized with fixed data. + +=head1 PARAMETERS + +=over 4 + +=item B<byte=>N + +Set the repeating byte to C<N>. The default is C<0xff> (all ones). + +Instead of C<byte=0> it is more efficient to use +L<nbdkit-null-plugin(1)>. + +=item [B<size=>]SIZE + +Specify the virtual size of the disk image. + +This parameter is optional. If omitted then a zero-sized device is +created. + +C<size=> is a magic config key and may be omitted in most cases. +See L<nbdkit(1)/Magic parameters>. + +=back + +=head1 FILES + +=over 4 + +=item F<$plugindir/nbdkit-ones-plugin.so> + +The plugin. + +Use C<nbdkit --dump-config> to find the location of C<$plugindir>. + +=back + +=head1 VERSION + +C<nbdkit-ones-plugin> first appeared in nbdkit 1.36. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-plugin(3)>, +L<nbdkit-data-plugin(1)>, +L<nbdkit-full-plugin(1)>, +L<nbdkit-null-plugin(1)>, +L<nbdkit-pattern-plugin(1)>, +L<nbdkit-random-plugin(1)>. +L<nbdkit-sparse-random-plugin(1)>, +L<nbdkit-zero-plugin(1)>. + +=head1 AUTHORS + +Richard W.M. Jones + +=head1 COPYRIGHT + +Copyright Red Hat diff --git a/plugins/pattern/nbdkit-pattern-plugin.pod b/plugins/pattern/nbdkit-pattern-plugin.pod index c5d4f0d36..f4ae324ed 100644 --- a/plugins/pattern/nbdkit-pattern-plugin.pod +++ b/plugins/pattern/nbdkit-pattern-plugin.pod @@ -11,7 +11,8 @@ nbdkit-pattern-plugin - plugin to serve a fixed pattern of data for testing C<nbdkit-pattern-plugin> is a plugin for L<nbdkit(1)> which serves a fixed pattern of data, read only. This is used for testing nbdkit filters and NBD clients. To create test disks filled with other -repeated patterns use L<nbdkit-data-plugin(1)> instead. +repeated patterns use L<nbdkit-data-plugin(1)> or +L<nbdkit-ones-plugin(1)> instead. The fixed pattern is the offset, as a 64 bit big endian integer, every 8 bytes. In hexadecimal this looks like: @@ -88,6 +89,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-data-plugin(1)>, L<nbdkit-full-plugin(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-offset-filter(1)>, L<nbdkit-random-plugin(1)>, L<nbdkit-sparse-random-plugin(1)>, diff --git a/plugins/random/nbdkit-random-plugin.pod b/plugins/random/nbdkit-random-plugin.pod index 040d86bfc..0d6da8d5f 100644 --- a/plugins/random/nbdkit-random-plugin.pod +++ b/plugins/random/nbdkit-random-plugin.pod @@ -76,6 +76,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-data-plugin(1)>, L<nbdkit-full-plugin(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-sparse-random-plugin(1)>, L<nbdkit-zero-plugin(1)>, diff --git a/plugins/sparse-random/nbdkit-sparse-random-plugin.pod b/plugins/sparse-random/nbdkit-sparse-random-plugin.pod index 03e98ee16..5636e76c4 100644 --- a/plugins/sparse-random/nbdkit-sparse-random-plugin.pod +++ b/plugins/sparse-random/nbdkit-sparse-random-plugin.pod @@ -110,6 +110,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-data-plugin(1)>, L<nbdkit-full-plugin(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-random-plugin(1)>, L<nbdkit-zero-plugin(1)>, diff --git a/plugins/zero/nbdkit-zero-plugin.pod b/plugins/zero/nbdkit-zero-plugin.pod index 03544ad6e..2ac846027 100644 --- a/plugins/zero/nbdkit-zero-plugin.pod +++ b/plugins/zero/nbdkit-zero-plugin.pod @@ -37,6 +37,7 @@ L<nbdkit-plugin(3)>, L<nbdkit-data-plugin(1)>, L<nbdkit-full-plugin(1)>, L<nbdkit-null-plugin(1)>, +L<nbdkit-ones-plugin(1)>, L<nbdkit-pattern-plugin(1)>, L<nbdkit-random-plugin(1)>, L<nbdkit-sparse-random-plugin(1)>. diff --git a/configure.ac b/configure.ac index dafddcc29..310de7ac1 100644 --- a/configure.ac +++ b/configure.ac @@ -95,6 +95,7 @@ non_lang_plugins="\ nbd \ null \ ondemand \ + ones \ partitioning \ pattern \ random \ @@ -1500,6 +1501,7 @@ AC_CONFIG_FILES([Makefile plugins/null/Makefile plugins/ocaml/Makefile plugins/ondemand/Makefile + plugins/ones/Makefile plugins/partitioning/Makefile plugins/pattern/Makefile plugins/perl/Makefile diff --git a/plugins/ones/Makefile.am b/plugins/ones/Makefile.am new file mode 100644 index 000000000..9f8cbc4e2 --- /dev/null +++ b/plugins/ones/Makefile.am @@ -0,0 +1,70 @@ +# nbdkit +# Copyright Red Hat +# +# 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-ones-plugin.pod + +plugin_LTLIBRARIES = nbdkit-ones-plugin.la + +nbdkit_ones_plugin_la_SOURCES = \ + ones.c \ + $(top_srcdir)/include/nbdkit-plugin.h \ + $(NULL) + +nbdkit_ones_plugin_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_builddir)/include \ + $(NULL) +nbdkit_ones_plugin_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_ones_plugin_la_LIBADD = \ + $(IMPORT_LIBRARY_ON_WINDOWS) \ + $(NULL) +nbdkit_ones_plugin_la_LDFLAGS = \ + -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \ + $(NULL) +if USE_LINKER_SCRIPT +nbdkit_ones_plugin_la_LDFLAGS += \ + -Wl,--version-script=$(top_srcdir)/plugins/plugins.syms +endif + +if HAVE_POD + +man_MANS = nbdkit-ones-plugin.1 +CLEANFILES += $(man_MANS) + +nbdkit-ones-plugin.1: nbdkit-ones-plugin.pod \ + $(top_builddir)/podwrapper.pl + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index 3c8123f45..3ae13d660 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -955,6 +955,10 @@ EXTRA_DIST += \ test-ondemand-locking.sh \ $(NULL) +# ones plugin test. +TESTS += test-ones.sh +EXTRA_DIST += test-ones.sh + # partitioning plugin test. TESTS += \ test-partitioning1.sh \ diff --git a/plugins/ones/ones.c b/plugins/ones/ones.c new file mode 100644 index 000000000..2ff1ed028 --- /dev/null +++ b/plugins/ones/ones.c @@ -0,0 +1,191 @@ +/* nbdkit + * Copyright Red Hat + * + * 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 <errno.h> + +#define NBDKIT_API_VERSION 2 + +#include <nbdkit-plugin.h> + +/* The size of disk in bytes (initialized by size=<SIZE> parameter). */ +static int64_t size = 0; + +/* Repeating byte. */ +static uint8_t byte = 0xff; + +static int +ones_config (const char *key, const char *value) +{ + int64_t r; + + if (strcmp (key, "size") == 0) { + r = nbdkit_parse_size (value); + if (r == -1) + return -1; + size = r; + } + else if (strcmp (key, "byte") == 0) { + if (nbdkit_parse_uint8_t ("byte", value, &byte) == -1) + return -1; + } + else { + nbdkit_error ("unknown parameter '%s'", key); + return -1; + } + + return 0; +} + +#define ones_config_help \ + "byte=<BYTE> Repeating byte to use (default: 0xff).\n" \ + "size=<SIZE> Size of the backing disk." + +/* Create the per-connection handle. */ +static void * +ones_open (int readonly) +{ + return NBDKIT_HANDLE_NOT_NEEDED; +} + +#define THREAD_MODEL NBDKIT_THREAD_MODEL_PARALLEL + +/* Get the disk size. */ +static int64_t +ones_get_size (void *handle) +{ + return size; +} + +/* Serves the same data over multiple connections. */ +static int +ones_can_multi_conn (void *handle) +{ + return 1; +} + +/* Cache. */ +static int +ones_can_cache (void *handle) +{ + /* Everything is already in memory, returning this without + * implementing .cache lets nbdkit do the correct no-op. + */ + return NBDKIT_CACHE_NATIVE; +} + +/* Read data. */ +static int +ones_pread (void *handle, void *buf, uint32_t count, uint64_t offset, + uint32_t flags) +{ + memset (buf, byte, count); + return 0; +} + +/* Write data. */ +static int +ones_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset, + uint32_t flags) +{ + /* nothing */ + return 0; +} + +/* Write zeroes. */ +static int +ones_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) +{ + /* nothing */ + return 0; +} + +/* Flush is a no-op, so advertise native FUA support */ +static int +ones_can_fua (void *handle) +{ + return NBDKIT_FUA_NATIVE; +} + +/* Trim. */ +static int +ones_trim (void *handle, uint32_t count, uint64_t offset, uint32_t flags) +{ + /* nothing */ + return 0; +} + +/* Nothing is persistent, so flush is trivially supported */ +static int +ones_flush (void *handle, uint32_t flags) +{ + return 0; +} + +/* Extents. */ +static int +ones_extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents *extents) +{ + /* Return a fully allocated data extent covering the entire disk. */ + return nbdkit_add_extent (extents, 0, size, 0); +} + +static struct nbdkit_plugin plugin = { + .name = "ones", + .version = PACKAGE_VERSION, + .config = ones_config, + .config_help = ones_config_help, + .magic_config_key = "size", + .open = ones_open, + .get_size = ones_get_size, + .can_multi_conn = ones_can_multi_conn, + .can_cache = ones_can_cache, + .pread = ones_pread, + .pwrite = ones_pwrite, + .zero = ones_zero, + .trim = ones_trim, + .can_fua = ones_can_fua, + .flush = ones_flush, + .extents = ones_extents, + /* In this plugin, errno is preserved properly along error return + * paths from failed system calls. + */ + .errno_is_preserved = 1, +}; + +NBDKIT_REGISTER_PLUGIN (plugin) diff --git a/tests/test-ones.sh b/tests/test-ones.sh new file mode 100755 index 000000000..1a2b22caf --- /dev/null +++ b/tests/test-ones.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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. + +source ./functions.sh +set -e +set -x + +requires_plugin ones +requires_nbdsh_uri +requires_nbdinfo + +# Check the disk contains 0xff by default. +nbdkit -U - ones 1024 --run ' +nbdsh -u "$uri" -c "assert (h.pread(1024,0) == b\"\\xff\"*1024)" +' + +# Change the byte. +nbdkit -U - ones 1024 byte=0x5a --run ' +nbdsh -u "$uri" -c "assert (h.pread(1024,0) == b\"\\x5a\"*1024)" +' + +# Check the disk is fully allocated. +nbdkit -U - ones 1G --run 'nbdinfo --map "$uri"' | \ + grep -E '0[[:space:]]+1073741824[[:space:]]+0[[:space:]]+data' -- 2.39.2
On Wed, May 17, 2023 at 11:06:58AM +0100, Richard W.M. Jones wrote:
Returns a fully allocated disk containing all 0xff (all ones), or another byte of your choice. --- + +=item B<byte=>N + +Set the repeating byte to C<N>. The default is C<0xff> (all ones). + +Instead of C<byte=0> it is more efficient to use +L<nbdkit-null-plugin(1)>.
Good catch. On v1, I suggested nbdkit-zero-plugin, but re-reading the difference between those two, I see you got the one I meant. (For the casual reader: zero-plugin has size zero, while the null plugin has non-zero size that reads as zero) Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, May 17, 2023 at 11:06:58AM +0100, Richard W.M. Jones wrote:
Returns a fully allocated disk containing all 0xff (all ones), or another byte of your choice. --- plugins/data/nbdkit-data-plugin.pod | 1 + plugins/full/nbdkit-full-plugin.pod | 1 + plugins/null/nbdkit-null-plugin.pod | 1 + plugins/ones/nbdkit-ones-plugin.pod | 77 +++++++ plugins/pattern/nbdkit-pattern-plugin.pod | 4 +- plugins/random/nbdkit-random-plugin.pod | 1 + .../nbdkit-sparse-random-plugin.pod | 1 + plugins/zero/nbdkit-zero-plugin.pod | 1 + configure.ac | 2 + plugins/ones/Makefile.am | 70 +++++++ tests/Makefile.am | 4 + plugins/ones/ones.c | 191 ++++++++++++++++++ tests/test-ones.sh | 53 +++++ 13 files changed, 406 insertions(+), 1 deletion(-)
+++ b/plugins/ones/ones.c @@ -0,0 +1,191 @@
+ +/* Write zeroes. */ +static int +ones_zero (void *handle, uint32_t count, uint64_t offset, uint32_t flags) +{ + /* nothing */ + return 0; +}
Since writing zeroes is a no-op...
+static struct nbdkit_plugin plugin = { + .name = "ones", + .version = PACKAGE_VERSION, + .config = ones_config, + .config_help = ones_config_help, + .magic_config_key = "size", + .open = ones_open, + .get_size = ones_get_size, + .can_multi_conn = ones_can_multi_conn, + .can_cache = ones_can_cache, + .pread = ones_pread, + .pwrite = ones_pwrite, + .zero = ones_zero, + .trim = ones_trim, + .can_fua = ones_can_fua, + .flush = ones_flush, + .extents = ones_extents, + /* In this plugin, errno is preserved properly along error return + * paths from failed system calls. + */ + .errno_is_preserved = 1, +};
...we probably also want to supply .can_fast_zero with a function that returns true. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
I pushed this series upstream, commits 3095d8bad..8627ed08b. The changes should address everything you mentioned: * nbdkit_parse_probability now only returns probabilities >= 0 and not NaN or infinite (but they can still be larger than 100%) * Add Rust bindings for parse_size, parse_bool and parse_probabilities * next_power_of_2 should now deal with all the corner cases that Eric described. * Document that log_2_bits must be called only with powers of 2. * Change 'rand' local variable to 'randnum' to avoid potential conflict with stdlib.h. * Use a local variable to store buf[i] in inner loop. * Add some suggestions for future enhancements to the filter made by Eric to the TODO list. * Add can_fast_zero to the ones filter. * Add R-b tags. * Re-test. Thanks! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit
This filter adds random data corruption when reading from the underlying plugin. --- filters/error/nbdkit-error-filter.pod | 4 + filters/evil/nbdkit-evil-filter.pod | 167 ++++++++++ configure.ac | 2 + filters/evil/Makefile.am | 77 +++++ tests/Makefile.am | 18 ++ filters/evil/evil.c | 422 ++++++++++++++++++++++++++ tests/test-evil-cosmic.sh | 77 +++++ tests/test-evil-large-p.sh | 55 ++++ tests/test-evil-small-p.sh | 57 ++++ tests/test-evil-stuck-high-bits.sh | 91 ++++++ tests/test-evil-stuck-low-bits.sh | 84 +++++ tests/test-evil-stuck-wires.sh | 85 ++++++ 12 files changed, 1139 insertions(+) diff --git a/filters/error/nbdkit-error-filter.pod b/filters/error/nbdkit-error-filter.pod index 49f21d10a..91af7f713 100644 --- a/filters/error/nbdkit-error-filter.pod +++ b/filters/error/nbdkit-error-filter.pod @@ -25,6 +25,9 @@ All parameters are optional, but you should usually specify one of the C<error-rate> or C<error-*-rate> parameters, B<otherwise this filter will do nothing>. +L<nbdkit-evil-filter(1)> is a related filter that injects data +corruption instead of errors. + =head1 EXAMPLES Inject a low rate of errors randomly into the connection: @@ -162,6 +165,7 @@ C<nbdkit-error-filter> first appeared in nbdkit 1.6. =head1 SEE ALSO L<nbdkit(1)>, +L<nbdkit-evil-filter(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-full-plugin(1)>, L<nbdkit-retry-filter(1)>, diff --git a/filters/evil/nbdkit-evil-filter.pod b/filters/evil/nbdkit-evil-filter.pod new file mode 100644 index 000000000..ff4010d33 --- /dev/null +++ b/filters/evil/nbdkit-evil-filter.pod @@ -0,0 +1,167 @@ +=head1 NAME + +nbdkit-evil-filter - add random data corruption to reads + +=head1 SYNOPSIS + + nbdkit --filter=evil PLUGIN [PLUGIN-ARGS...] + evil=[cosmic-rays|stuck-bits|stuck-wires] + [evil-probability=PROB] [evil-stuck-probability=PROB] + [evil-seed=SEED] + +=head1 DESCRIPTION + +nbdkit-evil-filter is a Byzantine filter for L<nbdkit(1)> that +randomly corrupts data when reading from the underlying plugin. This +can be used for testing filesystem checksums. Note that it does not +change write operations, so the underlying plugin contains the correct +data. + +L<nbdkit-error-filter(1)> is a related filter that injects hard errors +into the NBD protocol. + +This filter has several modes, controlled using the C<evil=...> +parameter. These are: + +=over 4 + +=item C<evil=cosmic-rays> + +Bits are flipped at random when reading data. The probability that a +bit is flipped is controlled using the C<evil-probability> parameter, +defaulting to 1e-8 (on average 1 in every 100 million bits read is +flipped). + +=item C<evil=stuck-bits> + +This is the default mode. + +Fixed bits in the backing file are stuck randomly high or low. The +C<evil-probability> parameter controls the expected probability that a +particular bit is stuck, defaulting in this mode to 1e-8 (1 in 100 +million). C<evil-stuck-probability> controls the probability that a +stuck bit is read as its stuck value or its correct value, defaulting +to 100% (always read as a stuck bit). + +=item C<evil=stuck-wires> + +This is similar to C<stuck-bits> but instead of simulating bad backing +data, it simulates stuck wires along the data path (eg. in a +register). The difference is that when reading, the stuck bit always +happens at the same position in the packet of data being read, +regardless of where on the underlying disk it is being read from. +C<evil-probability> and controls the probability of a stuck wire, +defaulting in this mode to 1e-6 (1 in 1 million). +C<evil-stuck-probability> controls the probability that a stuck bit is +read as its stuck value or its correct value, defaulting to 100% +(always read as a stuck bit). + +=back + +=head1 EXAMPLES + +Add some stuck bits to the backing file at random: + + nbdkit --filter=evil file disk.img + +Cosmic rays will flip (on average) one in every 100 million bits +copied from the backing file over NBD: + + nbdkit --filter=evil file disk.img evil=cosmic-rays \ + --run 'nbdcopy $uri output.img' + +=head1 NOTES + +=head2 Extents + +Plugins can be sparse. This filter only corrupts bits in non-sparse +parts of the backing disk and it leaves sparse regions unchanged +(which is realistic behaviour). If you wish to use this filter to +corrupt sparse regions, then combine this filter with +L<nbdkit-noextents-filter(1)>. For example: + + nbdkit --filter=evil --filter=noextents memory 1G + +=head2 Probability limited to [ 1e-12 .. 1/8 ] + +The current implementation limits probabilities to the range +S<[ 1e-12 .. 1/8 ]>. Values below this range are treated the same as +0%. Values above this range are treated the same as 100%. + +=head1 PARAMETERS + +=over 4 + +=item B<evil=cosmic-rays> + +=item B<evil=stuck-bits> + +=item B<evil=stuck-wires> + +Select the mode of evil. See the L</DESCRIPTION> above. The default +is C<stuck-bits>. + +=item B<evil-probability=>N + +=item B<evil-probability=>NB<:>M + +=item B<evil-probability=>NB<%> + +Set the probability for the mode. You can either use a floating point +number between 0 and 1, eg. C<evil-probability=0.001> or +C<evil-probability=1e-6>. Or you can write it as N in M, eg. +C<evil-probability=1:1000000> or C<evil-probability=3.33:100000>. Or +you can write this as a percentage, eg. C<evil-probability=1%>. + +The default probability depends on the mode. + +=item B<evil-seed=>SEED + +To make runs repeatable, use this to set a seed for the random number +generator. The default is to choose a seed at random. + +=item B<evil-stuck-probability=>N + +=item B<evil-stuck-probability=>NB<:>M + +=item B<evil-stuck-probability=>NB<%> + +For the "stuck-*" modes, the probability that when reading a stuck bit +you will read the stuck bit or the correct value. This defaults to 1 +(ie. 100%) which means the bit is always stuck. Setting it to 0.5 for +example will mean that half the time the bit appears stuck and half +the time you see the correct value. + +=back + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-evil-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-pause-filter> first appeared in nbdkit 1.36. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-delay-filter(1)>, +L<nbdkit-noextents-filter(1)>, +L<nbdkit-error-filter(1)>. + +=head1 AUTHORS + +Richard W.M. Jones + +=head1 COPYRIGHT + +Copyright Red Hat diff --git a/configure.ac b/configure.ac index 310de7ac1..18431958a 100644 --- a/configure.ac +++ b/configure.ac @@ -124,6 +124,7 @@ filters="\ exportname \ ext2 \ extentlist \ + evil \ fua \ gzip \ ip \ @@ -1534,6 +1535,7 @@ AC_CONFIG_FILES([Makefile filters/exportname/Makefile filters/ext2/Makefile filters/extentlist/Makefile + filters/evil/Makefile filters/fua/Makefile filters/gzip/Makefile filters/ip/Makefile diff --git a/filters/evil/Makefile.am b/filters/evil/Makefile.am new file mode 100644 index 000000000..ac6337428 --- /dev/null +++ b/filters/evil/Makefile.am @@ -0,0 +1,77 @@ +# nbdkit +# Copyright Red Hat +# +# 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-evil-filter.pod + +# Relies on a Unix domain socket. +if !IS_WINDOWS + +filter_LTLIBRARIES = nbdkit-evil-filter.la + +nbdkit_evil_filter_la_SOURCES = \ + evil.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_evil_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_builddir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_evil_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_evil_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(IMPORT_LIBRARY_ON_WINDOWS) \ + $(NULL) +nbdkit_evil_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \ + $(NULL) +if USE_LINKER_SCRIPT +nbdkit_evil_filter_la_LDFLAGS += \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms +endif + +if HAVE_POD + +man_MANS = nbdkit-evil-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-evil-filter.1: nbdkit-evil-filter.pod \ + $(top_builddir)/podwrapper.pl + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD +endif !IS_WINDOWS diff --git a/tests/Makefile.am b/tests/Makefile.am index 3ae13d660..9233c371e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1598,6 +1598,24 @@ EXTRA_DIST += \ test-error-triggered.sh \ $(NULL) +# evil filter test. +TESTS += \ + test-evil-cosmic.sh \ + test-evil-large-p.sh \ + test-evil-small-p.sh \ + test-evil-stuck-high-bits.sh \ + test-evil-stuck-low-bits.sh \ + test-evil-stuck-wires.sh \ + $(NULL) +EXTRA_DIST += \ + test-evil-cosmic.sh \ + test-evil-large-p.sh \ + test-evil-small-p.sh \ + test-evil-stuck-high-bits.sh \ + test-evil-stuck-low-bits.sh \ + test-evil-stuck-wires.sh \ + $(NULL) + # exitlast filter test. TESTS += test-exitlast.sh EXTRA_DIST += test-exitlast.sh diff --git a/filters/evil/evil.c b/filters/evil/evil.c new file mode 100644 index 000000000..a6e0a9ed2 --- /dev/null +++ b/filters/evil/evil.c @@ -0,0 +1,422 @@ +/* nbdkit + * Copyright Red Hat + * + * 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 <stdbool.h> +#include <stdint.h> +#include <inttypes.h> +#include <string.h> +#include <time.h> +#include <assert.h> + +#include <nbdkit-filter.h> + +#include "minmax.h" +#include "ispowerof2.h" +#include "random.h" + +enum mode { + COSMIC_RAYS, + STUCK_BITS, + STUCK_WIRES, +}; +static const char *evil_mode_to_string (enum mode); + +static enum mode evil_mode = STUCK_BITS; +static double evil_probability = -1; /* default depends on mode */ +static double evil_stuck_probability = 1.0; +static uint32_t evil_seed; + +/* Probabilities < ε are treated as zero to avoid both divide by zero + * problems and potentially exploding values in calculations. + */ +#define EPSILON 1e-12 + +/* Probabilities > MAXP are treated as 100%. This is because our + * algorithm below can corrupt at most 1 bit per byte and doesn't make + * progress otherwise. + */ +#define MAXP (1.0/8.0) + +static void +evil_load (void) +{ + evil_seed = time (NULL); +} + +static int +evil_config (nbdkit_next_config *next, nbdkit_backend *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "evil") == 0 || strcmp (key, "evil-mode") == 0) { + if (strcmp (value, "cosmic-rays") == 0 || + strcmp (value, "cosmic") == 0) { + evil_mode = COSMIC_RAYS; + return 0; + } + else if (strcmp (value, "stuck-bits") == 0 || + strcmp (value, "stuck-bit") == 0 || + strcmp (value, "stuck") == 0) { + evil_mode = STUCK_BITS; + return 0; + } + else if (strcmp (value, "stuck-wires") == 0 || + strcmp (value, "stuck-wire") == 0) { + evil_mode = STUCK_WIRES; + return 0; + } + else { + nbdkit_error ("evil: unknown mode: %s", value); + return -1; + } + } + else if (strcmp (key, "evil-probability") == 0) { + if (nbdkit_parse_probability ("evil-probability", value, + &evil_probability) == -1) + return -1; + if (evil_probability < 0 || evil_probability > 1) { + nbdkit_error ("%s: probability out of range, should be [0..1]", key); + return -1; + } + return 0; + } + else if (strcmp (key, "evil-stuck-probability") == 0) { + if (nbdkit_parse_probability ("evil-stuck-probability", value, + &evil_stuck_probability) == -1) + return -1; + if (evil_stuck_probability < 0 || evil_stuck_probability > 1) { + nbdkit_error ("%s: probability out of range, should be [0..1]", key); + return -1; + } + return 0; + } + else if (strcmp (key, "evil-seed") == 0) { + if (nbdkit_parse_uint32_t ("evil-seed", value, &evil_seed) == -1) + return -1; + return 0; + } + else + return next (nxdata, key, value); +} + +static int +evil_config_complete (nbdkit_next_config_complete *next, + nbdkit_backend *nxdata) +{ + if (evil_probability < 0) { + /* Choose default probability based on the chosen mode. */ + switch (evil_mode) { + case COSMIC_RAYS: + case STUCK_BITS: + evil_probability = 1e-8; + break; + case STUCK_WIRES: + evil_probability = 1e-6; + } + } + + return next (nxdata); +} + +#define evil_config_help \ + "evil=cosmic-rays|stuck-bits|stuck-wires\n" \ + " Set the mode (default: cosmic-rays).\n" \ + "evil-probability=PROB Probability of flipped or stuck bit.\n" \ + "evil-seed=SEED Random number seed.\n" \ + "evil-stuck-probability=PROB Probability of stuck bit being stuck." + +/* This is the heart of the algorithm, the function which corrupts + * the buffer after reading it from the plugin. + * + * The observation is that if we have a block of (eg) size 10**6 bits + * and our probability of finding a corrupt bit is (eg) 1/10**4, then + * we expect approximately 100 bits in the block to be corrupted. + * + * For stuck bits we want the corrupted bits to be the same on each + * access, either relative to the backing disk (STUCK_BITS) or to the + * request (STUCK_WIRES). + * + * Instead of creating an expensive bitmap ahead of time covering the + * whole disk, we can use the random number generator with a fixed + * seed derived from the offset of the start of the block. We can + * then choose a random number uniformly in the range [0..2*(1/P)] (in + * the example [0..2*10**4]) as the distance to the next corrupt bit. + * We jump forwards, corrupt that bit, and repeat until we reach the + * end of the block. + * + * "Corrupted" in this case can mean flipped by cosmic rays or stuck, + * depending on the filter mode. + * + * On average this will choose the right number of bits in the block. + * (Although their distribution will be suboptimal. In a uniform + * distribution it should be possible for two corrupted bits to be + * greater than 2*(1/P) apart, but the above algorithm would not do + * this. In practice this probably doesn't matter.) + * + * Note that "block" != "buffer", especially in the STUCK_BITS mode. + * We iterate over blocks as above, but only corrupt a bit when it + * happens to coincide with the buffer we have just read. + * + * We choose the block size adaptively so that at least 100 bits in + * the block will be corrupted. The block size must be a power of 2. + * The block size thus depends on the probability. + */ +enum corruption_type { FLIP, STUCK }; + +static uint64_t block_size; /* in bytes */ +static struct random_state state; /* only used for cosmic-rays */ + +static int +evil_thread_model (void) +{ + switch (evil_mode) { + case COSMIC_RAYS: + /* Because cosmic-rays uses the global random state we need to + * tighten the thread model. + */ + return NBDKIT_THREAD_MODEL_SERIALIZE_REQUESTS; + + case STUCK_BITS: + case STUCK_WIRES: + return NBDKIT_THREAD_MODEL_PARALLEL; + } + abort (); +} + +static int +evil_get_ready (int thread_model) +{ + switch (evil_mode) { + case COSMIC_RAYS: + xsrandom ((uint64_t) evil_seed, &state); + break; + + case STUCK_BITS: + case STUCK_WIRES: + ; + } + + /* Choose the block size based on the probability, so that at least + * 100 bits are expected to be corrupted in the block. Block size + * must be a power of 2. + * + * Example: P = 1e-4 + * => ideal block_size = 100 / 1e-4 = 1e6 (bits) = 1e6 / 8 (bytes) + * => next power of 2 block_size = 131072 = 2**17 + * => expected bits per block = ~104 + */ + if (evil_probability < EPSILON || evil_probability > MAXP) + block_size = 1024*1024; /* unused so value doesn't matter */ + else + block_size = next_power_of_2 ((uint64_t) (100. / evil_probability) / 8); + + nbdkit_debug ("evil: mode: %s, P: %lg, seed: %" PRIu32, + evil_mode_to_string (evil_mode), + evil_probability, evil_seed); + nbdkit_debug ("evil: block_size: %" PRIu64 " (2**%d)", + block_size, log_2_bits (block_size)); + nbdkit_debug ("evil: expected bits per block: %g", + 8 * block_size * evil_probability); + + return 0; +} + +static void corrupt_all_bits (uint8_t *buf, uint32_t count, + struct random_state *rs, + enum corruption_type ct); +static uint8_t corrupt_one_bit (uint8_t byte, unsigned bit, + uint64_t rand, enum corruption_type ct); + +static void +corrupt_buffer (uint8_t *buf, uint32_t count, uint64_t offset_in_block, + struct random_state *rs, enum corruption_type ct) +{ + /* No corruption, and avoids a divide by zero below. */ + if (evil_probability < EPSILON) return; + + /* 100% corruption, avoids lack of progress in the loop below. */ + if (evil_probability > MAXP) { + corrupt_all_bits (buf, count, rs, ct); + return; + } + + uint64_t offs, intvl, i, rand; + const uint64_t invp2 = (uint64_t) (2.0 / evil_probability); + + assert ((offset_in_block & ~(block_size-1)) == 0); + + /* Iterate over the whole block from the start. */ + for (offs = 0; offs < offset_in_block + count; ) { + /* Choose the length of the interval to the next corrupted bit, by + * picking a random number in [0..2*(1/P)]. + * + * Remember this is in bits! + */ + intvl = xrandom (rs) % invp2; + + /* Consume one more random state. We may or may not use this. + * But we need to always consume two random states per iteration + * to make the output predictable. + */ + rand = xrandom (rs); + + /* Adjust offs to that byte. */ + offs += intvl / 8; + + /* If we have gone past the end of buffer, stop. */ + if (offs >= offset_in_block + count) break; + + /* If the current offs lies within the buffer, corrupt a bit. */ + if (offs >= offset_in_block) { + i = offs - offset_in_block; + assert (i < count); + buf[i] = corrupt_one_bit (buf[i], intvl & 7, rand, ct); + } + } +} + +static void +corrupt_all_bits (uint8_t *buf, uint32_t count, + struct random_state *rs, enum corruption_type ct) +{ + size_t i; + unsigned bit; + uint64_t rand; + + /* This is used when MAXP < P <= 100%. We treat it the same as 100% + * and corrupt all bits. + */ + for (i = 0; i < count; ++i) { + for (bit = 0; bit < 8; ++bit) { + rand = xrandom (rs); + buf[i] = corrupt_one_bit (buf[i], bit, rand, ct); + } + } +} + +static uint8_t +corrupt_one_bit (uint8_t byte, unsigned bit, + uint64_t rand, enum corruption_type ct) +{ + const unsigned mask = 1 << bit; + + switch (ct) { + case FLIP: + byte ^= mask; + break; + case STUCK: + rand &= 0xffffffff; + if (evil_stuck_probability * 0x100000000 > rand) { + if (rand & 1) /* stuck high or low? */ + byte |= mask; + else + byte &= ~mask; + } + } + return byte; +} + +/* Read data. */ +static int +evil_pread (nbdkit_next *next, + void *handle, void *buf, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ + uint64_t seed, bstart, len; + struct random_state local_state; + + if (next->pread (next, buf, count, offset, flags, err) == -1) + return -1; + + switch (evil_mode) { + case COSMIC_RAYS: + /* Use the global random state because we want to flip bits at random. */ + corrupt_buffer (buf, count, 0, &state, FLIP); + break; + + case STUCK_BITS: + /* Split the request to align with blocks. */ + bstart = offset & ~(block_size-1); + while (count > 0) { + /* Set the seed so we corrupt the same bits relative to the offset. */ + seed = (int64_t) evil_seed + bstart; + xsrandom (seed, &local_state); + /* If the buffer straddles two blocks, shorten to just the part + * inside the first block. + */ + len = MIN (count, bstart + block_size - offset); + corrupt_buffer (buf, len, offset - bstart, &local_state, STUCK); + bstart += block_size; + offset += len; + buf += len; + count -= len; + } + break; + + case STUCK_WIRES: + /* Set the seed so we corrupt the same bits in every request. */ + seed = (int64_t) evil_seed; + xsrandom (seed, &local_state); + corrupt_buffer (buf, count, 0, &local_state, STUCK); + break; + } + + return 0; +} + +static const char * +evil_mode_to_string (enum mode mode) +{ + switch (mode) { + case COSMIC_RAYS: return "cosmic-rays"; + case STUCK_BITS: return "stuck-bits"; + case STUCK_WIRES: return "stuck-wires"; + } + abort (); +} + +static struct nbdkit_filter filter = { + .name = "evil", + .longname = "nbdkit evil filter", + .load = evil_load, + .config = evil_config, + .config_complete = evil_config_complete, + .config_help = evil_config_help, + .thread_model = evil_thread_model, + .get_ready = evil_get_ready, + .pread = evil_pread, +}; + +NBDKIT_REGISTER_FILTER (filter) diff --git a/tests/test-evil-cosmic.sh b/tests/test-evil-cosmic.sh new file mode 100755 index 000000000..2e5e6ab24 --- /dev/null +++ b/tests/test-evil-cosmic.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter with cosmic rays. + +source ./functions.sh +set -e +set -x + +requires_plugin null +requires_filter evil +requires_filter noextents +requires nbdcopy --version +requires $PYTHON --version + +f="test-evil-cosmic.out" +rm -f $f +cleanup_fn rm -f $f + +# 80 million zero bits in the backing disk, and the filter will +# randomly flip (ie. set high) 1 in 800,000 bits, or about 100. + +# XXX Actually the number of set bits clusters around 80. There could +# be a mistake in my calculations or the interval algorithm we use +# might be biased. + +export f +nbdkit -U - null 10000000 \ + --filter=evil --filter=noextents \ + evil=cosmic-rays evil-probability=1/800000 \ + --run 'nbdcopy "$uri" $f' + +# Count the number of bits set in the output file. Easier to use +# Python here ... + +$PYTHON -c ' +import os +fh = open(os.environ["f"], "rb") +buf = bytearray(fh.read()) +r = 0 +for b in buf: + if b != 0: + r += bin(b).count("1") + +print("non-zero bits: %d" % r) + +assert(r > 20 and r < 180) +' diff --git a/tests/test-evil-large-p.sh b/tests/test-evil-large-p.sh new file mode 100755 index 000000000..47d558678 --- /dev/null +++ b/tests/test-evil-large-p.sh @@ -0,0 +1,55 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter with large probabilities. + +source ./functions.sh +set -e +set -x + +requires_plugin null +requires_filter evil +requires_filter noextents +requires nbdcopy --version + +# This is the largest probability we support. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1/8 \ + --run 'nbdcopy "$uri" null:' + +# Anything larger is treated as 100%. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=0.5 \ + --run 'nbdcopy "$uri" null:' +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1.0 \ + --run 'nbdcopy "$uri" null:' diff --git a/tests/test-evil-small-p.sh b/tests/test-evil-small-p.sh new file mode 100755 index 000000000..377ef579b --- /dev/null +++ b/tests/test-evil-small-p.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter with small probabilities. + +source ./functions.sh +set -e +set -x + +requires_plugin null +requires_filter evil +requires_filter noextents +requires nbdcopy --version + +# Check absence of divide by zero errors. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=0 \ + --run 'nbdcopy "$uri" null:' + +# Smallest valid probability. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1e-12 \ + --run 'nbdcopy "$uri" null:' + +# Should be treated same as P = 0. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1e-13 \ + --run 'nbdcopy "$uri" null:' diff --git a/tests/test-evil-stuck-high-bits.sh b/tests/test-evil-stuck-high-bits.sh new file mode 100755 index 000000000..7af1bfdc8 --- /dev/null +++ b/tests/test-evil-stuck-high-bits.sh @@ -0,0 +1,91 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter in the default mode ("stuck-bits"). + +source ./functions.sh +set -e +set -x + +requires_plugin null +requires_filter evil +requires_filter noextents +requires_nbdsh_uri + +sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX) +pidfile=evil-stuck-high-bits.pid +files="$sock $pidfile" +rm -f $files +cleanup_fn rm -f $files + +# Run nbdkit with the evil filter. +start_nbdkit -P $pidfile -U $sock \ + --filter=evil --filter=noextents \ + null 1G evil-probability=1/800000 + +# Since 1 in 800,000 bits are stuck (on average), for every 100,000 +# bytes that we read we expect about 1 stuck bit. Note however that +# bits are stuck randomly low or high, and against the null filter you +# cannot see a stuck low bit, so in fact we expect to see only 1 stuck +# bit per 200,000 bytes. +# +# There is a separate test for stuck low bits (test-evil-stuck-low-bits.sh). +# +# Also stuck bits should be consistent across reads. + +nbdsh -u "nbd+unix://?socket=$sock" \ + -c - <<EOF +def count_bits(buf): + r = 0 + for i in range(0, len(buf)-1): + if buf[i] != 0: + r += bin(buf[i]).count("1") + return r + +def find_bit(buf): + for i in range(0, len(buf)-1): + if buf[i] != 0: + return i + return 0 + +# Expect about 50 stuck-high bits. +buf = h.pread(10000000, 0) +bits = count_bits(buf) +print("stuck high bits: %d (expected 50)" % bits) +assert(bits > 20 and bits < 80) + +# If we read subsets they should match the contents of the buffer. +i = find_bit(buf) +buf1 = h.pread(1000, i) +assert(buf1 == buf[i:i+1000]) + +EOF diff --git a/tests/test-evil-stuck-low-bits.sh b/tests/test-evil-stuck-low-bits.sh new file mode 100755 index 000000000..6f90b21ba --- /dev/null +++ b/tests/test-evil-stuck-low-bits.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter in the default mode ("stuck-bits"). + +source ./functions.sh +set -e +set -x + +requires_plugin ones +requires_filter evil +requires_nbdsh_uri + +sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX) +pidfile=evil-stuck-low-bits.pid +files="$sock $pidfile" +rm -f $files +cleanup_fn rm -f $files + +# Run nbdkit with the evil filter. +start_nbdkit -P $pidfile -U $sock \ + --filter=evil \ + ones 1G evil-probability=1/800000 + +# See description in test-evil-stuck-high-bits.sh. This test uses the +# ones plugin to test for stuck low bits. The other parameters are +# the same. + +nbdsh -u "nbd+unix://?socket=$sock" \ + -c - <<EOF +def count_bits(buf): + r = 0 + for i in range(0, len(buf)-1): + if buf[i] != 0xff: + r += 8 - bin(buf[i]).count("1") + return r + +def find_bit(buf): + for i in range(0, len(buf)-1): + if buf[i] != 0xff: + return i + return 0 + +# Expect about 50 stuck-low bits. +buf = h.pread(10000000, 32*1024*1024) +bits = count_bits(buf) +print("stuck low bits: %d (expected 50)" % bits) +assert(bits > 20 and bits < 80) + +# If we read subsets they should match the contents of the buffer. +i = find_bit(buf) +buf1 = h.pread(1000, 32*1024*1024 + i) +assert(buf1 == buf[i:i+1000]) + +EOF diff --git a/tests/test-evil-stuck-wires.sh b/tests/test-evil-stuck-wires.sh new file mode 100755 index 000000000..262b02049 --- /dev/null +++ b/tests/test-evil-stuck-wires.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright Red Hat +# +# 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 evil filter in stuck-wires mode. + +source ./functions.sh +set -e +set -x + +requires_plugin null +requires_filter evil +requires_filter noextents +requires_nbdsh_uri + +sock=$(mktemp -u /tmp/nbdkit-test-sock.XXXXXX) +pidfile=evil-stuck-wires.pid +files="$sock $pidfile" +rm -f $files +cleanup_fn rm -f $files + +# Run nbdkit with the evil filter. +start_nbdkit -P $pidfile -U $sock \ + --filter=evil --filter=noextents \ + null 1G evil=stuck-wires evil-probability=1/10000 + +# Reads from the filter should have 1:10,000 bits stuck high or low. +# However we don't see stuck low bits since we are always reading +# zeroes, so we only expect about 1:20,000 bits stuck high. +# +# If we read 10,000,000 bytes (80,000,000 bits) we would expect about +# 4000 stuck bits. +# +# No matter where we read from the pattern of stuck bits should be the +# same (stuck wires, not backing bits). + +nbdsh -u "nbd+unix://?socket=$sock" \ + -c - <<EOF +def count_bits(buf): + r = 0 + for i in range(0, len(buf)-1): + if buf[i] != 0: + r += bin(buf[i]).count("1") + return r + +buf1 = h.pread(10000000, 0) +bits = count_bits(buf1) +print("stuck high bits: %d (expected 4000)" % bits) +assert(bits > 3000 and bits < 5000) + +# These buffers should be identical. +buf2 = h.pread(10000000, 1024) +buf3 = h.pread(10000000, 32*1024*1024 - 9999) +assert(buf1 == buf2) +assert(buf1 == buf3) + +EOF -- 2.39.2
On Wed, May 17, 2023 at 11:06:59AM +0100, Richard W.M. Jones wrote:
This filter adds random data corruption when reading from the underlying plugin. ---
+=head1 VERSION + +C<nbdkit-pause-filter> first appeared in nbdkit 1.36.
Missed touching up this part of your copy-paste.
+static enum mode evil_mode = STUCK_BITS; +static double evil_probability = -1; /* default depends on mode */ +static double evil_stuck_probability = 1.0; +static uint32_t evil_seed; + +/* Probabilities < ε are treated as zero to avoid both divide by zero + * problems and potentially exploding values in calculations. + */ +#define EPSILON 1e-12 + +/* Probabilities > MAXP are treated as 100%. This is because our + * algorithm below can corrupt at most 1 bit per byte and doesn't make + * progress otherwise. + */ +#define MAXP (1.0/8.0)
These are reasonable constraints given your algorithm.
+ +/* This is the heart of the algorithm, the function which corrupts + * the buffer after reading it from the plugin. + * + * The observation is that if we have a block of (eg) size 10**6 bits + * and our probability of finding a corrupt bit is (eg) 1/10**4, then + * we expect approximately 100 bits in the block to be corrupted. + * + * For stuck bits we want the corrupted bits to be the same on each + * access, either relative to the backing disk (STUCK_BITS) or to the + * request (STUCK_WIRES). + * + * Instead of creating an expensive bitmap ahead of time covering the + * whole disk, we can use the random number generator with a fixed + * seed derived from the offset of the start of the block. We can + * then choose a random number uniformly in the range [0..2*(1/P)] (in + * the example [0..2*10**4]) as the distance to the next corrupt bit. + * We jump forwards, corrupt that bit, and repeat until we reach the + * end of the block. + * + * "Corrupted" in this case can mean flipped by cosmic rays or stuck, + * depending on the filter mode. + * + * On average this will choose the right number of bits in the block. + * (Although their distribution will be suboptimal. In a uniform + * distribution it should be possible for two corrupted bits to be + * greater than 2*(1/P) apart, but the above algorithm would not do + * this. In practice this probably doesn't matter.)
Likewise, in theory with a uniform distribution, it should be possible (although generally improbable) for any two adjacent bits to be corrupted, but in practice adjacent corruption is only possible for the last bit in one byte followed by the first bit in the next byte, since MAXP is chosen so that we always resume at the next byte after performing a corruption (so only 1 out of 8 positions is even a candidate for an adjacent corruption). One of the interesting points that comes out of the study of information theory and checksumming algorithms is the design of algorithms that can detect or even correct a small run of consecutively corrupt bits, since corruptions do tend to occur in bursts. For example, CRC32 [1] can detect 100% of bursts of < 32 consecutive bit corruptions if that is the only corruption in the packet (it takes either longer bursts, or more than one disjoint corruption, before you can run into a false negative where CRC says the overall packet was good despite the corruption). As written, the evil filter has a maximum burst of 1; maybe adding a maximum-burst parameter (find the bit position(s) to inject errors at, then randomly choose the burst size of the error at that point) might be a worthwhile future addition. But I don't see it as being essential to getting a first round of the filter committed. If we do add a burst parameter, you may also want to consider whether your current choice of a 50/50 stuck high vs. stuck low may need to consider more failure modes (entire burst stuck high, entire burst stuck low, all bits in the burst inverted, or a random number used as the xor mask for which bits in the burst to flip...). Do we care about cross-version compatibility (the same bits will be corrupt for a deterministic seed, regarless of future changes to the filter), or is it merely single-run consistency (for this build of nbdkit, a deterministic seed flips this particular set of bits, but the set of bits flipped may change in the next build of nbdkit when we start consuming randomness differently in order to account for more corruption modes). (I'd probably lean to explicitly documenting that we do not guarantee cross-version determinism; leaving the door open to consume randomness differently in the future.) [1] https://en.wikipedia.org/wiki/Cyclic_redundancy_check
+ +static int +evil_get_ready (int thread_model) +{ + switch (evil_mode) { + case COSMIC_RAYS: + xsrandom ((uint64_t) evil_seed, &state); + break; + + case STUCK_BITS: + case STUCK_WIRES: + ; + } + + /* Choose the block size based on the probability, so that at least + * 100 bits are expected to be corrupted in the block. Block size + * must be a power of 2. + * + * Example: P = 1e-4 + * => ideal block_size = 100 / 1e-4 = 1e6 (bits) = 1e6 / 8 (bytes) + * => next power of 2 block_size = 131072 = 2**17 + * => expected bits per block = ~104 + */ + if (evil_probability < EPSILON || evil_probability > MAXP) + block_size = 1024*1024; /* unused so value doesn't matter */ + else + block_size = next_power_of_2 ((uint64_t) (100. / evil_probability) / 8); + + nbdkit_debug ("evil: mode: %s, P: %lg, seed: %" PRIu32, + evil_mode_to_string (evil_mode), + evil_probability, evil_seed); + nbdkit_debug ("evil: block_size: %" PRIu64 " (2**%d)", + block_size, log_2_bits (block_size)); + nbdkit_debug ("evil: expected bits per block: %g", + 8 * block_size * evil_probability);
Definitely a useful addition over v1.
+ + return 0; +} + +static void corrupt_all_bits (uint8_t *buf, uint32_t count, + struct random_state *rs, + enum corruption_type ct); +static uint8_t corrupt_one_bit (uint8_t byte, unsigned bit, + uint64_t rand, enum corruption_type ct); + +static void +corrupt_buffer (uint8_t *buf, uint32_t count, uint64_t offset_in_block, + struct random_state *rs, enum corruption_type ct) +{ + /* No corruption, and avoids a divide by zero below. */ + if (evil_probability < EPSILON) return; + + /* 100% corruption, avoids lack of progress in the loop below. */ + if (evil_probability > MAXP) { + corrupt_all_bits (buf, count, rs, ct); + return; + } + + uint64_t offs, intvl, i, rand;
Since 'rand' is also reserved as the name of the <stdlib.h> function, you may want to pick a different name to this variable to avoid -Wshadow warnings.
+ const uint64_t invp2 = (uint64_t) (2.0 / evil_probability); + + assert ((offset_in_block & ~(block_size-1)) == 0); + + /* Iterate over the whole block from the start. */ + for (offs = 0; offs < offset_in_block + count; ) { + /* Choose the length of the interval to the next corrupted bit, by + * picking a random number in [0..2*(1/P)]. + * + * Remember this is in bits! + */ + intvl = xrandom (rs) % invp2; + + /* Consume one more random state. We may or may not use this. + * But we need to always consume two random states per iteration + * to make the output predictable. + */ + rand = xrandom (rs); + + /* Adjust offs to that byte. */ + offs += intvl / 8; + + /* If we have gone past the end of buffer, stop. */ + if (offs >= offset_in_block + count) break; + + /* If the current offs lies within the buffer, corrupt a bit. */ + if (offs >= offset_in_block) { + i = offs - offset_in_block; + assert (i < count); + buf[i] = corrupt_one_bit (buf[i], intvl & 7, rand, ct); + } + } +} + +static void +corrupt_all_bits (uint8_t *buf, uint32_t count, + struct random_state *rs, enum corruption_type ct) +{ + size_t i; + unsigned bit; + uint64_t rand; + + /* This is used when MAXP < P <= 100%. We treat it the same as 100% + * and corrupt all bits. + */ + for (i = 0; i < count; ++i) { + for (bit = 0; bit < 8; ++bit) { + rand = xrandom (rs); + buf[i] = corrupt_one_bit (buf[i], bit, rand, ct); + } + }
Is this actually corrupting all bits, or corrupting 1 bit per byte? I found the function name a bit confusing. (Doing XOR with -1 to invert all bits is not the same as choosing a random bit pattern but where only about 50% of the bits get flipped - but both behaviors can prove interesting to see how filesystems cope with such corruptions)
+} + +static uint8_t +corrupt_one_bit (uint8_t byte, unsigned bit, + uint64_t rand, enum corruption_type ct)
Another use of 'rand' that might be confusing given the <stdlib.h> function.
+{ + const unsigned mask = 1 << bit; + + switch (ct) { + case FLIP: + byte ^= mask; + break; + case STUCK: + rand &= 0xffffffff; + if (evil_stuck_probability * 0x100000000 > rand) { + if (rand & 1) /* stuck high or low? */ + byte |= mask; + else + byte &= ~mask; + } + } + return byte; +}
And I ran out of review time today, before finishing reading the rest of your patch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, May 17, 2023 at 05:32:50PM -0500, Eric Blake wrote:
On Wed, May 17, 2023 at 11:06:59AM +0100, Richard W.M. Jones wrote:
+static void +corrupt_all_bits (uint8_t *buf, uint32_t count, + struct random_state *rs, enum corruption_type ct) +{ + size_t i; + unsigned bit; + uint64_t rand; + + /* This is used when MAXP < P <= 100%. We treat it the same as 100% + * and corrupt all bits. + */ + for (i = 0; i < count; ++i) { + for (bit = 0; bit < 8; ++bit) { + rand = xrandom (rs); + buf[i] = corrupt_one_bit (buf[i], bit, rand, ct); + } + }
Is this actually corrupting all bits, or corrupting 1 bit per byte? I found the function name a bit confusing. (Doing XOR with -1 to invert all bits is not the same as choosing a random bit pattern but where only about 50% of the bits get flipped - but both behaviors can prove interesting to see how filesystems cope with such corruptions)
Isn't it corrupting each bit in turn, in effect corrupting all of them? However we could use a local variable to store buf[i] during the inner loop. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On Wed, May 17, 2023 at 11:06:59AM +0100, Richard W.M. Jones wrote:
This filter adds random data corruption when reading from the underlying plugin. ---
+++ b/tests/test-evil-small-p.sh @@ -0,0 +1,57 @@ + +# Check absence of divide by zero errors. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=0 \ + --run 'nbdcopy "$uri" null:' + +# Smallest valid probability. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1e-12 \ + --run 'nbdcopy "$uri" null:' + +# Should be treated same as P = 0. +nbdkit -U - -fv null 1M \ + --filter=evil --filter=noextents evil-probability=1e-13 \ + --run 'nbdcopy "$uri" null:' diff --git a/tests/test-evil-stuck-high-bits.sh b/tests/test-evil-stuck-high-bits.sh
Maybe worth capturing the output and grepping for the expected outputs, rather than requiring human post-parsing of the log file? Same for the large-p counterpart test. Not a show-stopper. At this point, I think you can go ahead and commit what you've cleaned up based on these reviews, and we can do any further cleanups as followups rather than needing to see a full v3 posting. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (3)
-
Eric Blake -
Laszlo Ersek -
Richard W.M. Jones