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

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(a)redhat.com&gt; >> --- >> 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