[nbdkit PATCH] eval: Allow user override of 'missing'
A comment in the code mentioned something that didn't actually work, but which can be useful for user-directed logging of what other callbacks they might want to implement. Signed-off-by: Eric Blake <eblake@redhat.com> --- I haven't pushed this one, becuase I'm not sure if we want it; but it was easy enough to whip together after an IRC question earlier today. plugins/eval/eval.c | 16 +++++++++++++--- plugins/eval/nbdkit-eval-plugin.pod | 6 ++++++ tests/test-eval.sh | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index 8f1eb6c..094cac5 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018-2019 Red Hat Inc. + * Copyright (C) 2018-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -169,6 +169,12 @@ create_script (const char *method, const char *value) return NULL; } + /* Special case for user override of missing */ + if (missing && strcmp (script, missing) == 0 && unlink (script) == -1) { + nbdkit_error ("unlink: %m"); + return NULL; + } + fp = fopen (script, "w"); if (fp == NULL) { nbdkit_error ("fopen: %s: %m", script); @@ -216,7 +222,7 @@ eval_load (void) /* To make things easier, create a "missing" script which always * exits with code 2. If a method is missing we call this script - * instead. It could even be overridden by the user. + * instead. It can even be overridden by the user. */ missing = create_script ("missing", "exit 2\n"); if (!missing) @@ -255,11 +261,15 @@ static int add_method (const char *key, const char *value) { char *script; + char *tmp = missing; /* Needed to allow user override of missing */ - if (get_script (key) != missing) { + missing = NULL; + if (get_script (key) != NULL) { + missing = tmp; nbdkit_error ("method %s defined more than once on the command line", key); return -1; } + missing = tmp; /* Do a bit of checking to make sure the key isn't malicious. This * duplicates work already done by nbdkit, but better safe than diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 88c1488..cbb4133 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -140,6 +140,12 @@ no longer see that key. All of these parameters are optional. +=item B<missing=>SCRIPT + +The parameter C<missing> defines a script that will be called in place +of any other callback not explicitly provided. If omitted, this +defaults to the script "exit 2". + =back =head1 ENVIRONMENT VARIABLES diff --git a/tests/test-eval.sh b/tests/test-eval.sh index 206c680..4557b02 100755 --- a/tests/test-eval.sh +++ b/tests/test-eval.sh @@ -42,7 +42,9 @@ cleanup_fn rm -f $files nbdkit -U - eval \ get_size='echo 64M' \ pread='dd if=/dev/zero count=$3 iflag=count_bytes' \ + missing='echo "in missing: $@" >> eval.out; exit 2' \ --run 'qemu-img info $nbd' > eval.out cat eval.out grep '67108864 bytes' eval.out +grep 'in missing' eval.out -- 2.24.1
On 2/10/20 1:44 PM, Eric Blake wrote:
A comment in the code mentioned something that didn't actually work, but which can be useful for user-directed logging of what other callbacks they might want to implement.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I haven't pushed this one, becuase I'm not sure if we want it; but it was easy enough to whip together after an IRC question earlier today.
plugins/eval/eval.c | 16 +++++++++++++--- plugins/eval/nbdkit-eval-plugin.pod | 6 ++++++ tests/test-eval.sh | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-)
Missing one hunk: diff --git i/plugins/eval/eval.c w/plugins/eval/eval.c index 31b62bb..094cac5 100644 --- i/plugins/eval/eval.c +++ w/plugins/eval/eval.c @@ -72,6 +72,7 @@ static const char *known_methods[] = { "flush", "get_size", "is_rotational", + "missing", "open", "pread", "pwrite", -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Mon, Feb 10, 2020 at 01:44:14PM -0600, Eric Blake wrote:
A comment in the code mentioned something that didn't actually work, but which can be useful for user-directed logging of what other callbacks they might want to implement.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I haven't pushed this one, becuase I'm not sure if we want it; but it was easy enough to whip together after an IRC question earlier today.
plugins/eval/eval.c | 16 +++++++++++++--- plugins/eval/nbdkit-eval-plugin.pod | 6 ++++++ tests/test-eval.sh | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/plugins/eval/eval.c b/plugins/eval/eval.c index 8f1eb6c..094cac5 100644 --- a/plugins/eval/eval.c +++ b/plugins/eval/eval.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2018-2019 Red Hat Inc. + * Copyright (C) 2018-2020 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -169,6 +169,12 @@ create_script (const char *method, const char *value) return NULL; }
+ /* Special case for user override of missing */ + if (missing && strcmp (script, missing) == 0 && unlink (script) == -1) { + nbdkit_error ("unlink: %m"); + return NULL; + } + fp = fopen (script, "w"); if (fp == NULL) { nbdkit_error ("fopen: %s: %m", script); @@ -216,7 +222,7 @@ eval_load (void)
/* To make things easier, create a "missing" script which always * exits with code 2. If a method is missing we call this script - * instead. It could even be overridden by the user. + * instead. It can even be overridden by the user. */ missing = create_script ("missing", "exit 2\n"); if (!missing) @@ -255,11 +261,15 @@ static int add_method (const char *key, const char *value) { char *script; + char *tmp = missing; /* Needed to allow user override of missing */
- if (get_script (key) != missing) { + missing = NULL; + if (get_script (key) != NULL) { + missing = tmp; nbdkit_error ("method %s defined more than once on the command line", key); return -1; } + missing = tmp;
This leaks the missing global? But yes the idea is fine, so ACK. Rich.
/* Do a bit of checking to make sure the key isn't malicious. This * duplicates work already done by nbdkit, but better safe than diff --git a/plugins/eval/nbdkit-eval-plugin.pod b/plugins/eval/nbdkit-eval-plugin.pod index 88c1488..cbb4133 100644 --- a/plugins/eval/nbdkit-eval-plugin.pod +++ b/plugins/eval/nbdkit-eval-plugin.pod @@ -140,6 +140,12 @@ no longer see that key.
All of these parameters are optional.
+=item B<missing=>SCRIPT + +The parameter C<missing> defines a script that will be called in place +of any other callback not explicitly provided. If omitted, this +defaults to the script "exit 2". + =back
=head1 ENVIRONMENT VARIABLES diff --git a/tests/test-eval.sh b/tests/test-eval.sh index 206c680..4557b02 100755 --- a/tests/test-eval.sh +++ b/tests/test-eval.sh @@ -42,7 +42,9 @@ cleanup_fn rm -f $files nbdkit -U - eval \ get_size='echo 64M' \ pread='dd if=/dev/zero count=$3 iflag=count_bytes' \ + missing='echo "in missing: $@" >> eval.out; exit 2' \ --run 'qemu-img info $nbd' > eval.out
cat eval.out grep '67108864 bytes' eval.out +grep 'in missing' eval.out -- 2.24.1
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
-- 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 2/10/20 3:10 PM, Richard W.M. Jones wrote:
On Mon, Feb 10, 2020 at 01:44:14PM -0600, Eric Blake wrote:
A comment in the code mentioned something that didn't actually work, but which can be useful for user-directed logging of what other callbacks they might want to implement.
Signed-off-by: Eric Blake <eblake@redhat.com> ---
I haven't pushed this one, becuase I'm not sure if we want it; but it was easy enough to whip together after an IRC question earlier today.
@@ -255,11 +261,15 @@ static int add_method (const char *key, const char *value) { char *script; + char *tmp = missing; /* Needed to allow user override of missing */
- if (get_script (key) != missing) { + missing = NULL; + if (get_script (key) != NULL) { + missing = tmp; nbdkit_error ("method %s defined more than once on the command line", key); return -1; } + missing = tmp;
This leaks the missing global?
Surprisingly not. We end up storing two malloc'd strings pointing to the same script name, one in 'missing' (which gets freed independently), and one in 'method_scripts[index_of_missing]' (which gets freed in our pass over method_scripts). But yes, I'll make sure to double-check a valgrind run before committing.
But yes the idea is fine, so ACK.
Rich.
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Richard W.M. Jones