[libvirt] Downloading and wiping assumes volume is a device or file

[Wido den Hollander]

Hi,

On 02/09/2012 09:27 PM, Eric Blake wrote:
> On 02/09/2012 01:04 PM, Wido den Hollander wrote:
>> Hi,
>>
>> I'm still working on the RBD (RADOS / Ceph) storage driver for libvirt
>> and I noticed the virStorageVolDownload and virStorageVolWipe methods.
>>
>> I assumed those would be passed on to the storage backend, but it doesn't.
>>
>> In the storageDriver the method storageVolumeDownload simply opens a
>> file descriptor and reads the device.
>>
>> Until now libvirt only had support for storage drivers who presented
>> regular files or block devices, but RBD doesn't. (Well, RBD could, but
>> I'm currently going for Qemu-RBD).
>>
>> In the future we might see more storage drivers in libvirt for a project
>> like Sheepdog as well.
>>
>> Sheepdog and RBD both have drivers in Qemu.
>>
>> What would the way be to approach this? Should the download, upload and
>> wipe methods be moved to the storage backends?
>>
>> There could also be an exception? If virStoragePoolType matches
>> VIR_STORAGE_POOL_RBD or VIR_STORAGE_POOL_SHEEPDOG the storage backend
>> could be invoked instead of opening the file descriptor?
>>
>> Any thoughts on this?

I was just checking out this code again and I saw that both RBD and 
Sheepdog prefix their device paths

RBD: rbd:pool/image
Sheepdog: sheepdog:image

In the storageDriver a check could be made. If 'path' is a file we use 
the regular methods. If it's a prefix, we send this down to the storage 
backend for further handling.

In short-term this doesn't brake any exisiting code and applications, 
but does allow these functions to work with new storage backends.

Wido

>
> For at least virStorageVolDownload, I can see two useful behaviors, and
> think we need to introduce a flag to specify which behavior:
>
> virStorageVolDownload(, 0) =>  verbatim copy of the raw volume, including
> metadata, as seen by the host; does not require any decryption keys to
> download, but may require secrets for accessing remote Sheepdog serves -
> great for cloning volumes
>
> virStorageVolDownload(, CONTENTS) =>  copy the contents of the volume as
> they would be seen in the guest; for any encrypted storage formats, this
> requires the right virSecret object to access the contents - great for
> converting arbitrary volumes into a raw storage format
>
> Conversely, virStorageVolUpload(, 0) installs a raw volume, and
> virStroageVolUpload(, CONTENTS) takes raw data and injects it into the
> contents that the guest will see (which may involve compression or
> encryption).
>
> Even on just local file systems, this would be the difference between
> 'cp qcow2.img1 qcow2.img2' (copy the entire metadata, without knowing
> the contents as seen by the guest) and 'qemu-img conviert -f qcow2 -O
> qcow2 qcow2.img1 qcow2.img2' (copy the entire guest contents, but by
> creating new qcow2 metadata, which may result in a smaller img2 file as
> abandoned sectors in img1 are elided).
>
> Very much worth doing!
>
> I also think that virStorageVolWipe should have a raw vs. contents, as in:
>
> virStorageVolWipe(, 0) =>  wipe the entire volume, including any storage
> metadata; a qcow2 file with 10M in use, 30M capacity, and occupying 20M
> on disk gets wiped into a raw file with 20M in use, capacity, and allocation
>
> virStorageVolWipe(, CONTENTS) =>  ensure that the guest sees an empty
> volume, but preserve host metadata; a qcow2 file with 10M in use, 30M
> capacity, and occupying 20M on disk gets converted to a qcow2 with 0
> bytes in use, 30M capacity; and we probably need a second flag to
> control whether disk usage is altered (that is, whether the image is
> thin or pre-allocated on disk)
>
> It certainly sounds like we should be wiring more of these decisions
> through to the backends, rather than attempting to do them all from the
> main storage driver.
>