[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/5] Add public APIs for storage volume upload/download



On Tue, Mar 15, 2011 at 10:37:01AM -0600, Eric Blake wrote:
> On 03/15/2011 06:30 AM, Daniel P. Berrange wrote:
> > New APIs are added allowing streaming of content to/from
> > storage volumes. A new API for creating volumes is also
> > added allowing the content to be provided immediately at
> > time of creation
> > 
> > * include/libvirt/libvirt.h.in: Add virStorageVolUpload and
> >   virStorageVolDownload, virStorageVolCreateUpload APIs
> 
> I see virStorageVol{Up,Down}load, but not virStorageVolCreateUpload in
> this patch.

As per previous posting, I removed VolCreateUpload since it isn't
needed and is complex to support.

> > +++ b/src/libvirt.c
> > @@ -9059,6 +9059,126 @@ error:
> >  
> >  
> >  /**
> > + * virStorageVolDownload:
> > + * @pool: pointer to volume to download
> > + * @stream: stream to use as output
> > + * @offset: position to start reading from
> > + * @length: limit on amount of data to download
> 
> Does 0 (or UINT64_MAX) have a special meaning of read-to-end?  If so,
> document that.

I've added docs about '0'

> > +
> > +    if (vol->conn->flags & VIR_CONNECT_RO ||
> > +        stream->conn->flags & VIR_CONNECT_RO) {
> > +        virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__);
> > +        goto error;
> > +    }
> 
> Why is reading a volume prohibited on a RO connection?

The data inside a storage volume may be security sensitive. Thus
we don't want readonly users to be able to read guest disks.

> > +/**
> > + * virStorageVolUpload:
> > + * @pool: pointer to volume to download
> > + * @stream: stream to use as output
> > + * @offset: position to start writing to
> > + * @length: limit on amount of data to upload
> 
> Again, is there any special meaning of length 0, or a directive to
> easily specify to end of input?

Added docs here.

> > + * @flags: flags for creation (unused, pass 0)
> 
> Do we need a flag to allow for expansion of the volume if the input
> stream (and length) are larger than the current size of the volume, for
> volumes that can be expanded in size?

I prefer to keep this side-effect free. We should have a separate
API for enlarging storage, particularly since everything except
plain raw files cannot do grow-on-demand.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]