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@redhat.com> --- 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 ....