[libvirt] [PATCH v3 05/31] Introduce virStreamSendHole

Michal Privoznik posted 31 patches 8 years, 8 months ago
[libvirt] [PATCH v3 05/31] Introduce virStreamSendHole
Posted by Michal Privoznik 8 years, 8 months ago
This API is used to tell the other side of the stream to skip
some bytes in the stream. This can be used to create a sparse
file on the receiving side of a stream.

It takes @length argument, which says how big the hole is. This
skipping is done from the current point of stream. Since our
streams are not rewindable like regular files, we don't need
@whence argument like seek(2) has.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 include/libvirt/libvirt-stream.h |  4 +++
 src/driver-stream.h              |  6 ++++
 src/libvirt-stream.c             | 61 ++++++++++++++++++++++++++++++++++++++++
 src/libvirt_public.syms          |  1 +
 4 files changed, 72 insertions(+)

diff --git a/include/libvirt/libvirt-stream.h b/include/libvirt/libvirt-stream.h
index bee25168b..14c9af142 100644
--- a/include/libvirt/libvirt-stream.h
+++ b/include/libvirt/libvirt-stream.h
@@ -50,6 +50,10 @@ int virStreamRecvFlags(virStreamPtr st,
                        size_t nbytes,
                        unsigned int flags);
 
+int virStreamSendHole(virStreamPtr st,
+                      long long length,
+                      unsigned int flags);
+
 
 /**
  * virStreamSourceFunc:
diff --git a/src/driver-stream.h b/src/driver-stream.h
index d4b048018..0a5201431 100644
--- a/src/driver-stream.h
+++ b/src/driver-stream.h
@@ -41,6 +41,11 @@ typedef int
                          size_t nbytes,
                          unsigned int flags);
 
+typedef int
+(*virDrvStreamSendHole)(virStreamPtr st,
+                        long long length,
+                        unsigned int flags);
+
 typedef int
 (*virDrvStreamEventAddCallback)(virStreamPtr stream,
                                 int events,
@@ -68,6 +73,7 @@ struct _virStreamDriver {
     virDrvStreamSend streamSend;
     virDrvStreamRecv streamRecv;
     virDrvStreamRecvFlags streamRecvFlags;
+    virDrvStreamSendHole streamSendHole;
     virDrvStreamEventAddCallback streamEventAddCallback;
     virDrvStreamEventUpdateCallback streamEventUpdateCallback;
     virDrvStreamEventRemoveCallback streamEventRemoveCallback;
diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
index 7535deb3c..a09896dcd 100644
--- a/src/libvirt-stream.c
+++ b/src/libvirt-stream.c
@@ -344,6 +344,67 @@ virStreamRecvFlags(virStreamPtr stream,
 }
 
 
+/**
+ * virStreamSendHole:
+ * @stream: pointer to the stream object
+ * @length: number of bytes to skip
+ * @flags: extra flags; not used yet, so callers should always pass 0
+ *
+ * Rather than transmitting empty file space, this API directs
+ * the @stream target to create @length bytes of empty space.
+ * This API would be used when uploading or downloading sparsely
+ * populated files to avoid the needless copy of empty file
+ * space.
+ *
+ * An example using this with a hypothetical file upload API
+ * looks like:
+ *
+ *   virStream st;
+ *
+ *   while (1) {
+ *     char buf[4096];
+ *     size_t len;
+ *     if (..in hole...) {
+ *       ..get hole size...
+ *       virStreamSendHole(st, len, 0);
+ *     } else {
+ *       ...read len bytes...
+ *       virStreamSend(st, buf, len);
+ *     }
+ *   }
+ *
+ * Returns 0 on success,
+ *        -1 error
+ */
+int
+virStreamSendHole(virStreamPtr stream,
+                  long long length,
+                  unsigned int flags)
+{
+    VIR_DEBUG("stream=%p, length=%lld flags=%x",
+              stream, length, flags);
+
+    virResetLastError();
+
+    virCheckStreamReturn(stream, -1);
+
+    if (stream->driver &&
+        stream->driver->streamSendHole) {
+        int ret;
+        ret = (stream->driver->streamSendHole)(stream, length, flags);
+        if (ret < 0)
+            goto error;
+        return ret;
+    }
+
+    virReportUnsupportedError();
+
+ error:
+    virDispatchError(stream->conn);
+    return -1;
+}
+
+
 /**
  * virStreamSendAll:
  * @stream: pointer to the stream object
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index d50b36a24..3be7cc6a0 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -762,6 +762,7 @@ LIBVIRT_3.1.0 {
 LIBVIRT_3.4.0 {
     global:
         virStreamRecvFlags;
+        virStreamSendHole;
 } LIBVIRT_3.1.0;
 
 # .... define new API here using predicted next version number ....
-- 
2.13.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 05/31] Introduce virStreamSendHole
Posted by John Ferlan 8 years, 8 months ago

On 05/16/2017 10:03 AM, Michal Privoznik wrote:
> This API is used to tell the other side of the stream to skip
> some bytes in the stream. This can be used to create a sparse
> file on the receiving side of a stream.
> 
> It takes @length argument, which says how big the hole is. This
> skipping is done from the current point of stream. Since our
> streams are not rewindable like regular files, we don't need
> @whence argument like seek(2) has.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  include/libvirt/libvirt-stream.h |  4 +++
>  src/driver-stream.h              |  6 ++++
>  src/libvirt-stream.c             | 61 ++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  1 +
>  4 files changed, 72 insertions(+)
> 

[...]

> diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
> index 7535deb3c..a09896dcd 100644
> --- a/src/libvirt-stream.c
> +++ b/src/libvirt-stream.c
> @@ -344,6 +344,67 @@ virStreamRecvFlags(virStreamPtr stream,
>  }
>  
>  
> +/**
> + * virStreamSendHole:
> + * @stream: pointer to the stream object
> + * @length: number of bytes to skip
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Rather than transmitting empty file space, this API directs
> + * the @stream target to create @length bytes of empty space.
> + * This API would be used when uploading or downloading sparsely
> + * populated files to avoid the needless copy of empty file
> + * space.
> + *
> + * An example using this with a hypothetical file upload API
> + * looks like:
> + *
> + *   virStream st;
> + *
> + *   while (1) {
> + *     char buf[4096];
> + *     size_t len;
> + *     if (..in hole...) {
> + *       ..get hole size...
> + *       virStreamSendHole(st, len, 0);
> + *     } else {
> + *       ...read len bytes...
> + *       virStreamSend(st, buf, len);
> + *     }
> + *   }
> + *
> + * Returns 0 on success,
> + *        -1 error
> + */
> +int
> +virStreamSendHole(virStreamPtr stream,
> +                  long long length,
> +                  unsigned int flags)
> +{
> +    VIR_DEBUG("stream=%p, length=%lld flags=%x",
> +              stream, length, flags);
> +
> +    virResetLastError();
> +
> +    virCheckStreamReturn(stream, -1);

Perhaps some preventative programming:

    virCheckNonNegativeArgGoto(length, error);

Although that would mean calling virDispatchError unless there was some
sort of virCheck*ArgReturn(length, -1)

Reviewed-by: John Ferlan <jferlan@redhat.com>


John

[...]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 05/31] Introduce virStreamSendHole
Posted by Michal Privoznik 8 years, 8 months ago
On 05/16/2017 11:13 PM, John Ferlan wrote:
> 
> 
> On 05/16/2017 10:03 AM, Michal Privoznik wrote:
>> This API is used to tell the other side of the stream to skip
>> some bytes in the stream. This can be used to create a sparse
>> file on the receiving side of a stream.
>>
>> It takes @length argument, which says how big the hole is. This
>> skipping is done from the current point of stream. Since our
>> streams are not rewindable like regular files, we don't need
>> @whence argument like seek(2) has.
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  include/libvirt/libvirt-stream.h |  4 +++
>>  src/driver-stream.h              |  6 ++++
>>  src/libvirt-stream.c             | 61 ++++++++++++++++++++++++++++++++++++++++
>>  src/libvirt_public.syms          |  1 +
>>  4 files changed, 72 insertions(+)
>>
> 
> [...]
> 
>> diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c
>> index 7535deb3c..a09896dcd 100644
>> --- a/src/libvirt-stream.c
>> +++ b/src/libvirt-stream.c
>> @@ -344,6 +344,67 @@ virStreamRecvFlags(virStreamPtr stream,
>>  }
>>  
>>  
>> +/**
>> + * virStreamSendHole:
>> + * @stream: pointer to the stream object
>> + * @length: number of bytes to skip
>> + * @flags: extra flags; not used yet, so callers should always pass 0
>> + *
>> + * Rather than transmitting empty file space, this API directs
>> + * the @stream target to create @length bytes of empty space.
>> + * This API would be used when uploading or downloading sparsely
>> + * populated files to avoid the needless copy of empty file
>> + * space.
>> + *
>> + * An example using this with a hypothetical file upload API
>> + * looks like:
>> + *
>> + *   virStream st;
>> + *
>> + *   while (1) {
>> + *     char buf[4096];
>> + *     size_t len;
>> + *     if (..in hole...) {
>> + *       ..get hole size...
>> + *       virStreamSendHole(st, len, 0);
>> + *     } else {
>> + *       ...read len bytes...
>> + *       virStreamSend(st, buf, len);
>> + *     }
>> + *   }
>> + *
>> + * Returns 0 on success,
>> + *        -1 error
>> + */
>> +int
>> +virStreamSendHole(virStreamPtr stream,
>> +                  long long length,
>> +                  unsigned int flags)
>> +{
>> +    VIR_DEBUG("stream=%p, length=%lld flags=%x",
>> +              stream, length, flags);
>> +
>> +    virResetLastError();
>> +
>> +    virCheckStreamReturn(stream, -1);
> 
> Perhaps some preventative programming:
> 
>     virCheckNonNegativeArgGoto(length, error);
> 
> Although that would mean calling virDispatchError unless there was some
> sort of virCheck*ArgReturn(length, -1)

Just like flags are checked on the receiving side, I think this should
be checked in receiving side too. From arguments sanity POV, this is
just like any other API call - the receiving side has to do the sanity
check while the sender side merely grabs whatever has been passed and
sends to the receiving side. Yes, there are few exceptions to this rule,
esp. obvious misuse of APIs, like passing NULL as pointer to a libvirt
object, negative size of an passed array and so on.

This approach has one great advantage - we can have an older client
talking to newer server and still achieve the same functionality as in
new-new case. For instance, imagine we make some sense of negative
seeks, @length < 0. If we check it just on the daemon, then yay - you
can still use older client in order to send negative seek.

Michal

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