On 04/20/2017 06:01 AM, Michal Privoznik wrote: > This is just a wrapper over new function that have been just > introduced: virStreamSkip() . It's very similar to > virStreamSendAll() except it handles sparse streams well. You could have some changes here due to previous API name changes. Still the commit msg is a bit terse needs some cleanup anyway. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > include/libvirt/libvirt-stream.h | 59 +++++++++++++++++++-- > src/libvirt-stream.c | 107 +++++++++++++++++++++++++++++++++++++++ > src/libvirt_public.syms | 1 + > 3 files changed, 164 insertions(+), 3 deletions(-) > > diff --git a/include/libvirt/libvirt-stream.h b/include/libvirt/libvirt-stream.h > index e5f5126..3e9ff11 100644 > --- a/include/libvirt/libvirt-stream.h > +++ b/include/libvirt/libvirt-stream.h > @@ -69,9 +69,9 @@ int virStreamHoleSize(virStreamPtr, > * @nbytes: size of the data array > * @opaque: optional application provided data > * > - * The virStreamSourceFunc callback is used together > - * with the virStreamSendAll function for libvirt to > - * obtain the data that is to be sent. > + * The virStreamSourceFunc callback is used together with > + * the virStreamSendAll and virStreamSparseSendAll functions > + * for libvirt to obtain the data that is to be sent. > * > * The callback will be invoked multiple times, > * fetching data in small chunks. The application > @@ -95,6 +95,59 @@ int virStreamSendAll(virStreamPtr st, > void *opaque); > > /** > + * virStreamSourceHoleFunc: > + * @st: the stream object > + * @inData: are we in data section > + * @length: how long is the section we are currently in > + * @opaque: optional application provided data > + * > + * The virStreamSourceHoleFunc callback is used together > + * with the virStreamSparseSendAll function for libvirt to > + * obtain the length of section stream is currently in. > + * > + * Moreover, upon successful return, @length should be > + * updated with how much bytes are there left until current s/much/many s/there// s/until/until the/ > + * section ends (be it data section or hole section) and if s/be it/either/ s/) and if/). Also if/ > + * the stream is currently in data section, @inData should > + * be set to a non-zero value and vice versa. Need extra line between paragraphs > + * As a corner case, there's an implicit hole at the end of s/As a corner case,/NB:/ > + * each file. If that's the case, @inData should be set to 0 > + * as well as @length. ... @inData and @length should both be set to 0. (reads better to me) Need extra line between paragraphs > + * Moreover, this function should upon its return leave the > + * file in the position it was called with. Consider instead: This function is not expected to adjust the current position within the file. (or more explicitly - should not adjust!) > + * > + * Returns 0 on success, > + * -1 upon error > + */ > +typedef int (*virStreamSourceHoleFunc)(virStreamPtr st, > + int *inData, > + unsigned long long *length, > + void *opaque); > + > +/** > + * virStreamSourceSkipFunc: > + * @st: the stream object > + * @length: stream hole size > + * @opaque: optional application provided data > + * > + * This callback is used together with the virStreamSparseSendAll > + * to skip holes in the underlying file as reported by > + * virStreamSourceHoleFunc. The callback may be invoked multiple times as holes are found during processing a stream. The application should skip processing the hole in the stream source and then return. A return value of -1 at any time will abort the send operation. > + * > + * Returns 0 on success, > + * -1 upon error. > + */ > +typedef int (*virStreamSourceSkipFunc)(virStreamPtr st, > + unsigned long long length, > + void *opaque); > + > +int virStreamSparseSendAll(virStreamPtr st, > + virStreamSourceFunc handler, > + virStreamSourceHoleFunc holeHandler, > + virStreamSourceSkipFunc skipHandler, > + void *opaque); > + > +/** > * virStreamSinkFunc: > * > * @st: the stream object > diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c > index 81190cc..7ad5a38 100644 > --- a/src/libvirt-stream.c > +++ b/src/libvirt-stream.c > @@ -565,7 +565,114 @@ virStreamSendAll(virStreamPtr stream, > } > > > + Spurious extra line... Now there's 3 between functions. > /** > + * virStreamSparseSendAll: > + * @stream: pointer to the stream object > + * @handler: source callback for reading data from application > + * @holeHandler: source callback for determining holes > + * @skipHandler: skip holes as reported by @holeHandler > + * @opaque: application defined data > + * > + * Some dummy description here. Some smart-aleck comment here ;-) > + * > + * Opaque data in @opaque are shared between @handler, @holeHandler and @skipHandler. > + * > + * Returns 0 if all the data was successfully sent. The caller > + * should invoke virStreamFinish(st) to flush the stream upon > + * success and then virStreamFree. > + * > + * Returns -1 upon any error, with virStreamAbort() already > + * having been called, so the caller need only call > + * virStreamFree(). > + */ > +int virStreamSparseSendAll(virStreamPtr stream, > + virStreamSourceFunc handler, > + virStreamSourceHoleFunc holeHandler, > + virStreamSourceSkipFunc skipHandler, > + void *opaque) > +{ > + char *bytes = NULL; > + size_t want = VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX; > + int ret = -1; > + unsigned long long dataLen = 0; > + > + VIR_DEBUG("stream=%p handler=%p holeHandler=%p opaque=%p", > + stream, handler, holeHandler, opaque); > + > + virResetLastError(); > + > + virCheckStreamReturn(stream, -1); > + virCheckNonNullArgGoto(handler, cleanup); > + virCheckNonNullArgGoto(holeHandler, cleanup); > + virCheckNonNullArgGoto(skipHandler, cleanup); > + > + if (stream->flags & VIR_STREAM_NONBLOCK) { > + virReportError(VIR_ERR_OPERATION_INVALID, "%s", > + _("data sources cannot be used for non-blocking streams")); > + goto cleanup; > + } > + > + if (VIR_ALLOC_N(bytes, want) < 0) > + goto cleanup; > + > + for (;;) { > + int inData, got, offset = 0; > + unsigned long long sectionLen; > + > + if (!dataLen) { > + if (holeHandler(stream, &inData, §ionLen, opaque) < 0) { > + virStreamAbort(stream); > + goto cleanup; > + } > + > + if (!inData && sectionLen) { So if !inData and sectionLen == 0, we don't go here, but > + if (virStreamSkip(stream, sectionLen) < 0) { > + virStreamAbort(stream); > + goto cleanup; > + } > + > + if (skipHandler(stream, sectionLen, opaque) < 0) { > + virReportSystemError(errno, "%s", > + _("unable to skip hole")); > + virStreamAbort(stream); > + goto cleanup; > + } > + continue; > + } else { > + dataLen = sectionLen; We go here indicating 'dataLen = 0' which is a misnomer since we're not inData, rather we at EOF, right? Or did I miss something a bit subtle. > + } > + } > + > + if (want > dataLen) > + want = dataLen; > + > + got = (handler)(stream, bytes, want, opaque); > + if (got < 0) { > + virStreamAbort(stream); > + goto cleanup; > + } > + if (got == 0) > + break; > + while (offset < got) { > + int done; > + done = virStreamSend(stream, bytes + offset, got - offset); > + if (done < 0) > + goto cleanup; > + offset += done; > + dataLen -= done; > + } > + } > + ret = 0; > + > + cleanup: > + VIR_FREE(bytes); > + > + if (ret != 0) > + virDispatchError(stream->conn); > + > + return ret; > +}/** Need to clean up the spacing here... e.g. } /** Another one close for an ACK - some cleanup here as well as seeing the fallout from previous patches. John > * virStreamRecvAll: > * @stream: pointer to the stream object > * @handler: sink callback for writing data to application > diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms > index 008dc59..ea4ddd5 100644 > --- a/src/libvirt_public.syms > +++ b/src/libvirt_public.syms > @@ -765,6 +765,7 @@ LIBVIRT_3.3.0 { > virStreamRecvFlags; > virStreamSkip; > virStreamSparseRecvAll; > + virStreamSparseSendAll; > } LIBVIRT_3.1.0; > > # .... define new API here using predicted next version number .... > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list