[libvirt] [PATCH v2 4/6] fdstream: Add internal callback on stream close

[Eric Blake]

On 12/07/2011 11:08 AM, Peter Krempa wrote:
> This patch adds another callback to a FDstream object. The original
> callback is used by the daemon stream driver to handle events.
> 
> This callback is called if and only if the stream is about to be closed.
> This might be used to handle cleanup steps after a fdstream exits. This
> will be used later on in ensuring mutualy exclusive access to consoles.
> 
> * src/fdstream.c:
>         - emit the callback, when stream is being closed
>         - add data structures needed to handle the callback
>         - add function to register callback
> * src/fdstream.h:
>         - define function prototypes for the callback
> ---
>  src/fdstream.c |   39 ++++++++++++++++++++++++++++++++++-----
>  src/fdstream.h |   11 +++++++++++
>  2 files changed, 45 insertions(+), 5 deletions(-)
> 

> @@ -381,7 +394,6 @@ retry:
>      return ret;
>  }
> 
> -
>  static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes)
>  {
>      struct virFDStreamData *fdst = st->privateData;
> @@ -431,7 +443,6 @@ retry:
>      return ret;
>  }
> 
> -
>  static virStreamDriver virFDStreamDrv = {
>      .streamSend = virFDStreamWrite,
>      .streamRecv = virFDStreamRead,
> @@ -479,14 +490,12 @@ static int virFDStreamOpenInternal(virStreamPtr st,
>      return 0;
>  }
> 
> -
>  int virFDStreamOpen(virStreamPtr st,
>                      int fd)
>  {
>      return virFDStreamOpenInternal(st, fd, NULL, -1, 0);
>  }
> 
> -

Why all the whitespace deletion?

Again, the patch looks sane, but I'd feel more comfortable with Dan's
review.

> +++ b/src/fdstream.h
> @@ -26,6 +26,13 @@
>  # include "internal.h"
>  # include "command.h"
> 
> +/* internal callback, the generic one is used up by daemon stream driver */
> +/* the close callback is called with fdstream private data locked */
> +typedef void (*virFDStreamInternalCloseCb)(virStreamPtr st, void *opaque);
> +
> +typedef void (*virFDStreamInternalCloseCbFreeOpaque)(void *opaque);

Do we need a new typedef, or can we reuse the same signature as the
generic streams one, and just make our streams callback (which we feed
to the stream driver) then invoke the user's callback of the same signature?

> +
> +
>  int virFDStreamOpen(virStreamPtr st,
>                      int fd);
> 
> @@ -45,4 +52,8 @@ int virFDStreamCreateFile(virStreamPtr st,
>                            int oflags,
>                            mode_t mode);
> 
> +int virFDStreamSetInternalCloseCb(virStreamPtr st,
> +                                  virFDStreamInternalCloseCb cb,
> +                                  void *opaque,
> +                                  virFDStreamInternalCloseCbFreeOpaque fcb);

The name 'Internal' may scare users away from trying to use this.  But
I'm not sure of a better naming scheme.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111213/ed8d6e16/attachment-0001.sig>