Re: [libvirt] Extensions to the libvirt Storage API

On 07/28/2010 05:08 AM, Daniel P. Berrange wrote: >On Tue, Jul 27, 2010 at 06:28:01PM -0500, Patrick Dignan wrote: > >>Hi Everyone, >> >>I'm looking at implementing some functionality in libvirt that would >>allow it to call functions in an unpublished iSCSI library. Some of the >>functionality I wish to implement is not currently part of the libvirt >>storage API. I wanted to suggest the following additions to the storage >>API: grow volumes, show whether thin provisioning is enabled, enable >>thin provisioning, disable thin provisioning, create snapshots, and >>delete snapshots. I've added a patch at the end of the mail showing how >>I think these functions should be implemented. Note that I have not >>included details about the virStorageSnapshotDefPtr yet, that's the next >>step. >> >>Perhaps this should be in a separate mail for better threading, but it >>seems a bit strange to me that the storage interface isn't pluggable in >>the traditional sense. In order to add a backend to libvirt, one has to >>make modifications all over the place, for example: virt-inst, the >>Makefile.am, the configure.ac, storage_backend.h, and several other >>places. It would make sense to me to make this pluggable such that >>someone could just load in a library that implements the required >>functions and some identifying information (eg type of storage, >>description, etc). A list of supported backends could be stored in >>empty files in a directory somewhere, or some similar hack. This way >>someone could write a plugin for tgtd for example, or in my case the >>library I'm working with. I think this would also help others with >>writing plugins for more storage backends. How difficult do you think >>this would be? I'm willing to do a reasonable amount of work to get >>this implemented, but I want to know what the experts think! >> >We explicitly don't support external driver plugins in libvirt for a >couple of reasons > > - We don't want to support use of closed source plugins > - We don't want to guarentee stability of any aspect of > libvirt's internal API > >We would like to see support for the various vendor specific iSCSI >extensions to allow volume creation/deletion, but want that code to >be part of the libvirt codebase. > > Understandable. I was thinking that there is currently no way to specify a vendor of a storage backend. For example, an iSCSI vendor. This makes it look like implementing vendor-specific extensions requires creating a new backend, even though there's already an iSCSI backend. It seems like a secondary field for vendor, and maybe even model, could help this.

>> /* File creation/cloning functions used for cloning between backends */ >> int virStorageBackendCreateRaw(virConnectPtr conn, >>@@ -76,6 +83,12 @@ >> virStorageBackendCreateVol createVol; >> virStorageBackendRefreshVol refreshVol; >> virStorageBackendDeleteVol deleteVol; >>+ virStorageBackendGrowVol growVol; >> >I'd call this 'resizeVol' since there's no reason we can't also support >shrinking. > Do all backends support shrinking? I was under the impression shrinking is not quite a universal feature, so it made sense to me to break this out. If most backends support shrinking, it makes sense to use resizeVol instead then.

>>+ virStorageBackendThinProvisionShow thinProvisionShow; >>+ virStorageBackendThinProvisionEnable thinProvisionEnable; >>+ virStorageBackendThinProvisionDisable thinProvisionDisable; >> >I'm not really liking this as a concept. The other storage drivers, and >indeed my iSCSI server, deal with thin provisioning on a per-volume basis >when creating the volume. The libvirt model is that if in the XML, then ><allocation> value is zero then the user is requesting thin provisioning >of that volume. ie no storage allocated for it upfront. If<allocation> >matches<capacity> then the volume should be fully allocated upfront. > > Sorry, that was a bit of a misnomer on my part, these functions are intended to be used at the volume level. However, what if they want to enable thin provisioning or disable it after the fact? Is that not going to be supported? An example use case of enabling after the fact is if they want to enable it, and then grow the volume to a larger size. Disabling, of course, could be done at any time (eg for a speed increase). Do you object to the thinProvisionShow function, in either case?

>>+ virStorageBackendCreateSnapshot createSnapshot; >>+ virStorageBackendDeleteSnapshot deleteSnapshot; >> >There's no need for snapshot APIs. This functionality is already supported >via the normal volume creation API, just specify the original volume to be >snapshotted in the XML as the backing store. > I wasn't aware of this functionality. It looks like it's implemented on a per-hypervisor basis. It'd be really cool to get snapshotting integrated into storage backends with snapshotting support, so that snapshots would show up in both libvirt and the storage backend's UI, but I can see how this would be nearly impossible.