[libvirt] [PATCH v3 29/31] Introduce virStorageVol{Download, Upload}Flags

[Michal Privoznik]

On 05/17/2017 05:42 PM, John Ferlan wrote:
> 
> 
> On 05/16/2017 10:04 AM, Michal Privoznik wrote:
>> These flags to APIs will tell if caller wants to use sparse
>> stream for storage transfer. At the same time, it's safe to
>> enable them in storage driver frontend and rely on our backends
>> checking the flags. This way we can enable specific flags only on
>> some specific backends, e.g. enable
>> VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM for filesystem backend but
>> not iSCSI backend.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
>>  include/libvirt/libvirt-storage.h |  9 +++++++++
>>  src/libvirt-storage.c             |  4 ++--
>>  src/remote/remote_protocol.x      |  2 ++
>>  src/storage/storage_driver.c      |  4 ++--
>>  src/storage/storage_util.c        | 10 ++++++----
>>  5 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
>> index 45ec72065..4517f713c 100644
>> --- a/include/libvirt/libvirt-storage.h
>> +++ b/include/libvirt/libvirt-storage.h
>> @@ -346,11 +346,20 @@ virStorageVolPtr        virStorageVolCreateXMLFrom      (virStoragePoolPtr pool,
>>                                                           const char *xmldesc,
>>                                                           virStorageVolPtr clonevol,
>>                                                           unsigned int flags);
>> +
>> +typedef enum {
>> +    VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM = 1 << 0, /* Use sparse stream */
>> +} virStorageVolDownloadFlags;
>> +
>>  int                     virStorageVolDownload           (virStorageVolPtr vol,
>>                                                           virStreamPtr stream,
>>                                                           unsigned long long offset,
>>                                                           unsigned long long length,
>>                                                           unsigned int flags);
>> +typedef enum {
>> +    VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM = 1 << 0,  /* Use sparse stream */
>> +} virStorageVolUploadFlags;
>> +
> 
> /me wonders should the backend specific concerns be described in
> comments prior to each enum or is that too specific. Maybe it's more of
> a 'specific backends' that perform "file based manipulation" (rather
> than block based)...  I dunno.  I'll leave it to you though - the more
> documentation now while it's fresh in your mind the better.
> 
> 
>>  int                     virStorageVolUpload             (virStorageVolPtr vol,
>>                                                           virStreamPtr stream,
>>                                                           unsigned long long offset,
>> diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
>> index 05eec8a9d..64202998b 100644
>> --- a/src/libvirt-storage.c
>> +++ b/src/libvirt-storage.c
>> @@ -1549,7 +1549,7 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
>>   * @stream: stream to use as output
>>   * @offset: position in @vol to start reading from
>>   * @length: limit on amount of data to download
>> - * @flags: extra flags; not used yet, so callers should always pass 0
>> + * @flags: bitwise-OR of virStorageVolDownloadFlags
>>   *
>>   * Download the content of the volume as a stream. If @length
>>   * is zero, then the remaining contents of the volume after
>> @@ -1613,7 +1613,7 @@ virStorageVolDownload(virStorageVolPtr vol,
>>   * @stream: stream to use as input
>>   * @offset: position to start writing to
>>   * @length: limit on amount of data to upload
>> - * @flags: extra flags; not used yet, so callers should always pass 0
>> + * @flags: bitwise-OR of virStorageVolUploadFlags
>>   *
>>   * Upload new content to the volume from a stream. This call
>>   * will fail if @offset + @length exceeds the size of the
> 
> I suppose for each you c(sh)ould have documented what the specific FLAG
> does and the expectations therein..

How about this?

iff --git i/src/libvirt-storage.c w/src/libvirt-storage.c
index 64202998b..35f9926d5 100644
--- i/src/libvirt-storage.c
+++ w/src/libvirt-storage.c
@@ -1555,6 +1555,13 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
  * is zero, then the remaining contents of the volume after
  * @offset will be downloaded.
  *
+ * If VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM is set in @flags
+ * effective transmission of holes is enabled. This assumes using
+ * the @stream with combination of virStreamSparseRecvAll() or
+ * virStreamRecvFlags(stream, ..., flags =
+ * VIR_STREAM_RECV_STOP_AT_HOLE) for honouring holes sent by
+ * server.
+ *
  * This call sets up an asynchronous stream; subsequent use of
  * stream APIs is necessary to transfer the actual data,
  * determine how much data is successfully transferred, and
@@ -1621,6 +1628,11 @@ virStorageVolDownload(virStorageVolPtr vol,
  * will be raised if an attempt is made to upload greater
  * than @length bytes of data.
  *
+ * If VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM is set in @flags
+ * effective transmission of holes is enabled. This assumes using
+ * the @stream with combination of virStreamSparseSendAll() or
+ * virStreamSendHole() to preserve source file sparseness.
+ *
  * This call sets up an asynchronous stream; subsequent use of
  * stream APIs is necessary to transfer the actual data,
  * determine how much data is successfully transferred, and

Michal