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@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;
+
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
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 87b2bd365..25e62a181 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -4896,6 +4896,7 @@ enum remote_procedure {
/**
* @generate: both
* @writestream: 1
+ * @sparseflag: VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM
* @acl: storage_vol:data_write
*/
REMOTE_PROC_STORAGE_VOL_UPLOAD = 208,
@@ -4903,6 +4904,7 @@ enum remote_procedure {
/**
* @generate: both
* @readstream: 1
+ * @sparseflag: VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM
* @acl: storage_vol:data_read
*/
REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209,
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2103ed11d..1b0d776c7 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2117,7 +2117,7 @@ storageVolDownload(virStorageVolPtr obj,
virStorageVolDefPtr vol = NULL;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM, -1);
if (!(vol = virStorageVolDefFromVol(obj, &pool, &backend)))
return -1;
@@ -2285,7 +2285,7 @@ storageVolUpload(virStorageVolPtr obj,
virStorageVolStreamInfoPtr cbdata = NULL;
int ret = -1;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM, -1);
if (!(vol = virStorageVolDefFromVol(obj, &pool, &backend)))
return -1;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 908cad874..493c651b7 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -2401,8 +2401,9 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
char *target_path = vol->target.path;
int ret = -1;
int has_snap = 0;
+ bool sparse = flags & VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM, -1);
/* if volume has target format VIR_STORAGE_FILE_PLOOP
* we need to restore DiskDescriptor.xml, according to
* new contents of volume. This operation will be perfomed
@@ -2427,7 +2428,7 @@ virStorageBackendVolUploadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
/* Not using O_CREAT because the file is required to already exist at
* this point */
ret = virFDStreamOpenBlockDevice(stream, target_path,
- offset, len, false, O_WRONLY);
+ offset, len, sparse, O_WRONLY);
cleanup:
VIR_FREE(path);
@@ -2447,8 +2448,9 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
char *target_path = vol->target.path;
int ret = -1;
int has_snap = 0;
+ bool sparse = flags & VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM;
- virCheckFlags(0, -1);
+ virCheckFlags(VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM, -1);
if (vol->target.format == VIR_STORAGE_FILE_PLOOP) {
has_snap = storageBackendPloopHasSnapshots(vol->target.path);
if (has_snap < 0) {
@@ -2465,7 +2467,7 @@ virStorageBackendVolDownloadLocal(virConnectPtr conn ATTRIBUTE_UNUSED,
}
ret = virFDStreamOpenBlockDevice(stream, target_path,
- offset, len, false, O_RDONLY);
+ offset, len, sparse, O_RDONLY);
cleanup:
VIR_FREE(path);
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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..
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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?
>
Fine -
Reviewed-by: John Ferlan <jferlan@redhat.com>
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.