[v2] Allow sparse streams for block devices

[PATCH v2 07/17] libvirt-storage: Document volume upload/download stream format

Posted by Michal Privoznik 5 years, 7 months ago

For libvirt, the volume is just a binary blob and it doesn't
interpret data on volume upload/download. But as it turns out,
this unspoken assumption is not clear to our users. Document it
explicitly.

Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libvirt-storage.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index a45c8b98c1..8738f6dd14 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -1590,7 +1590,9 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
  *
  * Download the content of the volume as a stream. If @length
  * is zero, then the remaining contents of the volume after
- * @offset will be downloaded.
+ * @offset will be downloaded. Please note, that the data is
+ * not interpreted and therefore data received by stream
+ * client are in the very same format the volume is in.
  *
  * If VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM is set in @flags
  * effective transmission of holes is enabled. This assumes using
@@ -1663,7 +1665,9 @@ virStorageVolDownload(virStorageVolPtr vol,
  * will fail if @offset + @length exceeds the size of the
  * volume. Otherwise, if @length is non-zero, an error
  * will be raised if an attempt is made to upload greater
- * than @length bytes of data.
+ * than @length bytes of data. Please note, that the data is
+ * not interpreted and therefore data sent by stream client
+ * must be in the very same format the volume is in.
  *
  * If VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM is set in @flags
  * effective transmission of holes is enabled. This assumes using
-- 
2.26.2

Re: [PATCH v2 07/17] libvirt-storage: Document volume upload/download stream format

Posted by Peter Krempa 5 years, 5 months ago

On Tue, Jul 07, 2020 at 21:46:25 +0200, Michal Privoznik wrote:
> For libvirt, the volume is just a binary blob and it doesn't
> interpret data on volume upload/download. But as it turns out,
> this unspoken assumption is not clear to our users. Document it
> explicitly.
> 
> Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libvirt-storage.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
> index a45c8b98c1..8738f6dd14 100644
> --- a/src/libvirt-storage.c
> +++ b/src/libvirt-storage.c
> @@ -1590,7 +1590,9 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
>   *
>   * Download the content of the volume as a stream. If @length
>   * is zero, then the remaining contents of the volume after
> - * @offset will be downloaded.
> + * @offset will be downloaded. Please note, that the data is
> + * not interpreted and therefore data received by stream
> + * client are in the very same format the volume is in.

I don't think this wording clarifies it that much as it's not obvious
what's meant by 'interpreted'.

How about:

Please note that the stream transports the volume itself as is, so the
downloaded data may not correspond to guest OS visible state in cases
when a complex storage format such as qcow2 or vmdk is used.

Re: [PATCH v2 07/17] libvirt-storage: Document volume upload/download stream format

Posted by Michal Privoznik 5 years, 5 months ago

On 8/20/20 10:35 AM, Peter Krempa wrote:
> On Tue, Jul 07, 2020 at 21:46:25 +0200, Michal Privoznik wrote:
>> For libvirt, the volume is just a binary blob and it doesn't
>> interpret data on volume upload/download. But as it turns out,
>> this unspoken assumption is not clear to our users. Document it
>> explicitly.
>>
>> Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>   src/libvirt-storage.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
>> index a45c8b98c1..8738f6dd14 100644
>> --- a/src/libvirt-storage.c
>> +++ b/src/libvirt-storage.c
>> @@ -1590,7 +1590,9 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
>>    *
>>    * Download the content of the volume as a stream. If @length
>>    * is zero, then the remaining contents of the volume after
>> - * @offset will be downloaded.
>> + * @offset will be downloaded. Please note, that the data is
>> + * not interpreted and therefore data received by stream
>> + * client are in the very same format the volume is in.
> 
> I don't think this wording clarifies it that much as it's not obvious
> what's meant by 'interpreted'.
> 
> How about:
> 
> Please note that the stream transports the volume itself as is, so the
> downloaded data may not correspond to guest OS visible state in cases
> when a complex storage format such as qcow2 or vmdk is used.
> 

Fine by me.

Michal

Re: [PATCH v2 07/17] libvirt-storage: Document volume upload/download stream format

Posted by Peter Krempa 5 years, 5 months ago

On Thu, Aug 20, 2020 at 14:29:21 +0200, Michal Privoznik wrote:
> On 8/20/20 10:35 AM, Peter Krempa wrote:
> > On Tue, Jul 07, 2020 at 21:46:25 +0200, Michal Privoznik wrote:
> > > For libvirt, the volume is just a binary blob and it doesn't
> > > interpret data on volume upload/download. But as it turns out,
> > > this unspoken assumption is not clear to our users. Document it
> > > explicitly.
> > > 
> > > Suggested in: https://bugzilla.redhat.com/show_bug.cgi?id=1851023#c17
> > > 
> > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > > ---
> > >   src/libvirt-storage.c | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
> > > index a45c8b98c1..8738f6dd14 100644
> > > --- a/src/libvirt-storage.c
> > > +++ b/src/libvirt-storage.c
> > > @@ -1590,7 +1590,9 @@ virStorageVolCreateXMLFrom(virStoragePoolPtr pool,
> > >    *
> > >    * Download the content of the volume as a stream. If @length
> > >    * is zero, then the remaining contents of the volume after
> > > - * @offset will be downloaded.
> > > + * @offset will be downloaded. Please note, that the data is
> > > + * not interpreted and therefore data received by stream
> > > + * client are in the very same format the volume is in.
> > 
> > I don't think this wording clarifies it that much as it's not obvious
> > what's meant by 'interpreted'.
> > 
> > How about:
> > 
> > Please note that the stream transports the volume itself as is, so the
> > downloaded data may not correspond to guest OS visible state in cases
> > when a complex storage format such as qcow2 or vmdk is used.
> > 
> 
> Fine by me.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

> 
> Michal