Re: [PATCH 05/27] Introduce virStreamRegisterSkip and virStreamSkipCallback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/28/2016 04:04 AM, Michal Privoznik wrote:
> The former is a public API and registers a callback that
> will be called whenever the other side of a stream calls
> virStreamSkip. The latter is a wrapper that actually calls
> the callback. It is not made public as it is intended to be
> used purely internally.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---

> +/**
> + * virStreamSkipFunc:
> + * @stream: stream
> + * @offset: size of hole in bytes

Again, naming this 'length' makes more sense (you're not skipping TO a
particular offset, but OVER a given length).

> +++ b/src/libvirt-stream.c
> @@ -286,6 +286,76 @@ virStreamRecv(virStreamPtr stream,
>  
>  
>  /**
> + * virStreamRegisterSkip:
> + * @stream: stream
> + * @skipCb: callback function
> + * @opaque: optional application provided data
> + *
> + * This function registers callback that will be called whenever

s/callback/a callback/

> + * the other side of the @stream is willing to skip a hole in the
> + * stream.
> + *
> + * Returns 0 on success,
> + *        -1 otherwise.
> + */
> +int
> +virStreamRegisterSkip(virStreamPtr stream,
> +                      virStreamSkipFunc skipCb,
> +                      void *opaque)
> +{
> +    VIR_DEBUG("stream=%p, skipCb=%p opaque=%p", stream, skipCb, opaque);
> +
> +    virResetLastError();
> +
> +    virCheckStreamReturn(stream, -1);
> +    virCheckNonNullArgReturn(skipCb, -1);
> +
> +    if (stream->skipCb) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("A skip callback is already registered"));
> +        return -1;

I guess that means we allow passing skipCb=NULL to deregister a
callback; does it need to be specifically documented?  Are there
scenarios where you WANT to deregister before closing something else, to
make sure that a stale callback is not called during a race scenario?

> +int
> +virStreamSkipCallback(virStreamPtr stream,
> +                      unsigned long long offset)
> +{
> +    VIR_DEBUG("stream=%p, offset=%llu", stream, offset);
> +
> +    virCheckStreamReturn(stream, -1);
> +
> +    if (stream->skipCb) {
> +        int ret;
> +        ret = (stream->skipCb)(stream, offset, stream->skipCbOpaque);

I might have omitted the () around stream->skipCb, but I don't know if
we have a consistent style, and yours makes it obvious that we know we
are dereferencing a function pointer.

> +++ b/src/libvirt_public.syms
> @@ -735,6 +735,7 @@ LIBVIRT_1.3.3 {
>  LIBVIRT_1.3.5 {
>      global:
>          virStreamSkip;
> +        virStreamRegisterSkip;

Worth keeping sorted?

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

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]