On 05/17/2017 12:30 PM, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
>>> ---
>>> 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?
>
Fine -
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
