Libguestfs - Mar 2020 - Re: [PATCH nbdkit] server: Only display "close: " debug message if callback is called.

For example if .preconnect fails, it printed the debug messages:

  debug: preconnect: ...
  debug: close: ...

This confused me into thinking that the close callback was actually
being called without a corresponding open.  In fact it is not called
in this case.  Suppress the debug message unless the callback is
actually being called.
---
 server/backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/backend.c b/server/backend.c
index 51b56a48..108f4a25 100644
--- a/server/backend.c
+++ b/server/backend.c
@@ -241,10 +241,10 @@ backend_close (struct backend *b)
   struct handle *h = get_handle (conn, b->i);
 
   /* outer-to-inner order, opposite .open */
-  controlpath_debug ("%s: close", b->name);
 
   if (h->handle) {
     assert (h->state & HANDLE_OPEN);
+    controlpath_debug ("%s: close", b->name);
     b->close (b, h->handle);
   }
   else
-- 
2.25.0

On 3/4/20 9:15 AM, Richard W.M. Jones wrote:
> For example if .preconnect fails, it printed the debug messages:
> 
>    debug: preconnect: ...
>    debug: close: ...
> 
> This confused me into thinking that the close callback was actually
> being called without a corresponding open.  In fact it is not called
> in this case.  Suppress the debug message unless the callback is
> actually being called.
> ---
>   server/backend.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
Makes sense, but .finalize should get the same treatment since it 
likewise gets skipped when .prepare fails.
> 
> diff --git a/server/backend.c b/server/backend.c
> index 51b56a48..108f4a25 100644
> --- a/server/backend.c
> +++ b/server/backend.c
> @@ -241,10 +241,10 @@ backend_close (struct backend *b)
>     struct handle *h = get_handle (conn, b->i);
>   
>     /* outer-to-inner order, opposite .open */
> -  controlpath_debug ("%s: close", b->name);
>   
>     if (h->handle) {
>       assert (h->state & HANDLE_OPEN);
> +    controlpath_debug ("%s: close", b->name);
>       b->close (b, h->handle);
>     }
>     else
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Wed, Mar 04, 2020 at 09:57:12AM -0600, Eric Blake
wrote:
> On 3/4/20 9:15 AM, Richard W.M. Jones wrote:
> >For example if .preconnect fails, it printed the debug messages:
> >
> >   debug: preconnect: ...
> >   debug: close: ...
> >
> >This confused me into thinking that the close callback was actually
> >being called without a corresponding open.  In fact it is not called
> >in this case.  Suppress the debug message unless the callback is
> >actually being called.
> >---
> >  server/backend.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Makes sense, but .finalize should get the same treatment since it
> likewise gets skipped when .prepare fails.
Thanks, I pushed this with the same change made to .finalize too.

Rich.
> >
> >diff --git a/server/backend.c b/server/backend.c
> >index 51b56a48..108f4a25 100644
> >--- a/server/backend.c
> >+++ b/server/backend.c
> >@@ -241,10 +241,10 @@ backend_close (struct backend *b)
> >    struct handle *h = get_handle (conn, b->i);
> >    /* outer-to-inner order, opposite .open */
> >-  controlpath_debug ("%s: close", b->name);
> >    if (h->handle) {
> >      assert (h->state & HANDLE_OPEN);
> >+    controlpath_debug ("%s: close", b->name);
> >      b->close (b, h->handle);
> >    }
> >    else
> >
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/