On Tue, Aug 13, 2019 at 10:41:27AM -0500, Eric Blake wrote:
On 8/13/19 10:12 AM, Richard W.M. Jones wrote:
Commit 0904dd113dfa36485623b3c1756dc1aeab3dddeb removed the theoretical possibility of clearing the debug callback from the handle by setting it to NULL (although that was dubious from an API point of view).
This commit adds an explicit way to do this. Another advantage of this is we can internally call this API from other places when we want to clear/free the debug callback. --- generator/generator | 11 +++++++++++ lib/debug.c | 17 ++++++++++++++--- lib/handle.c | 4 +--- 3 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/generator/generator b/generator/generator index 6ccfc5f..d76eeea 100755 --- a/generator/generator +++ b/generator/generator @@ -1013,6 +1013,17 @@ a handle then messages are printed on C<stderr>.
The callback should not call C<nbd_*> APIs on the same handle since it can be called while holding the handle lock and will cause a deadlock."; +}; + + "clear_debug_callback", { + default_call with + args = []; + ret = RErr;
Is RErr the right type, or can we make this a 'never fails' function?
Yes this can never fail. We don't have a way to say that (except RUInt but that's a bit different), and I'm a bit dubious about adding one because it limits our choices in future.
+ shortdesc = "clear the debug callback"; + longdesc = "\ +Remove the debug callback if one was previously associated +with the handle (with C<nbd_set_debug_callback>). If not
s/not/no/
Oops, fixed in my copy.
+callback was associated this does nothing."; };
"set_handle_name", { diff --git a/lib/debug.c b/lib/debug.c index ad4d9cb..c1decb2 100644 --- a/lib/debug.c +++ b/lib/debug.c @@ -38,13 +38,24 @@ nbd_unlocked_get_debug (struct nbd_handle *h) return h->debug; }
+int +nbd_unlocked_clear_debug_callback (struct nbd_handle *h) +{ + if (h->debug_callback) + /* ignore return value */ + h->debug_callback (LIBNBD_CALLBACK_FREE, h->debug_data, NULL, NULL); + h->debug_callback = NULL; + h->debug_data = NULL; + return 0; +}
Is it worth returning -1 if no callback was associated, and/or forwarding the return value of callback(FREE, ptr) as the return value of this function? (That's more complicated; I'm also fine with always returning 0 and ignoring the callback(FREE) return).
I think it's probably best to return 0 always. Reduces complexity for callers if they don't have to worry if a debug function was previously registered or not. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org