[libvirt] [PATCH 05/27] Introduce virStreamRegisterSkip and virStreamSkipCallback

[Michal Privoznik]

On 05.05.2016 16:49, Eric Blake wrote:
> 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 at redhat.com>
>> ---
> 
>> +/**
>> + * 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?

I haven't thought of that. I mean, the line just above this check
prohibits skipCB being NULL. But it seems like this will be thrown away
anyway.

> 
>> +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?
> 

Sure.

Michal