You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Marcus Sorensen <sh...@gmail.com> on 2012/12/10 20:06:11 UTC
pluggable storage implementation
Just curious, hadn't thought about this before but it seems that at least
on KVM (probably similar in Xen and VMware too?), there are two separate
issues with storage in the existing code. First, adding a new storage type
is a matter of adding in a new 'else if' or something in a bunch of
different places, as well as tweaking behavior to match the storage type.
Second, everything about the storage is tightly integrated with Libvirt,
meaning that if your storage type is not supported by Libvirt it's much,
much more difficult to implement.
Are these both being addressed by the storage changes, for example can we
write a storage plugin that creates pools/volumes that libvirt doesn't know
about and still attach those to instances? Or would we need to patch
libvirt to utilize our storage first?
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
Alright, I reworked the SnapshotCommand related commands that extended it
to use the actual StoragePool rather than only passing a uuid, and changed
the few spots in the server code that was creating new commands. I didn't
change any existing accessors (just added getPool()) so the various
existing agent code shouldn't break. This seemed to be the cleanest way to
do it, and made them consistent with the other storage commands.
The only thing I'm stuck on now is the getStoragePoolByURI and whether to
hard code that to libvirt or break that out entirely and put it in
LibvirtComputingResource. I'm tempted to do the latter since it simply
parses the URI and then calls createStoragePool. Then future secondary
storage types can be accounted for by parsing the storage type out of the
URI.
Once I've got these all done and run some basic tests to ensure everything
works I'll post it in a review request so you can see all of the changes.
On Tue, Dec 11, 2012 at 11:51 AM, Marcus Sorensen <sh...@gmail.com>wrote:
> Here's a list according to what's in LibvirtComputingResource of things
> that would need to either pass a pool type or a StorageFilerTO:
>
> CreateVolumeFromSnapshotCommand
> CreatePrivateTemplateFromVolumeCommand
> PrimaryStorageDownloadCommand
>
> Shall I overload these to allow an optional pool type to be passed, so as
> not to break or have to rework the other hypervisor types?
>
>
> On Tue, Dec 11, 2012 at 10:59 AM, Marcus Sorensen <sh...@gmail.com>wrote:
>
>> There are also some commands that don't pass StorageFilerTO, but just a
>> uuid of a storage pool (primaryStoragePoolNameLabel), I'm assuming those
>> will need to be reworked to pass either a type or the whole StorageFilerTO?
>>
>>
>> On Tue, Dec 11, 2012 at 10:45 AM, Marcus Sorensen <sh...@gmail.com>wrote:
>>
>>> There are a few snags, there are times when we get the storage pool by
>>> asking LibvirtStorageAdaptor, in which case I don't know the adaptor type
>>> to pass to KVMStoragePoolManager. For example:
>>>
>>> secondaryStoragePool =
>>> _storagePoolMgr.getStoragePoolByURI(secondaryStorageUrl);
>>>
>>> In this case it parses the url and then runs createStoragePool on it,
>>> returning the resulting pool. It seems these need a separate strategy
>>> regarding pulling secondary storage out of Libvirt... do we just hard-code
>>> these to libvirt for now, do we need to push the parsing of the url into
>>> LibvirtComputingResource, or do you have any suggestions?
>>>
>>>
>>>
>>> On Mon, Dec 10, 2012 at 3:27 PM, Marcus Sorensen <sh...@gmail.com>wrote:
>>>
>>>> Ok, so you do in fact want to change all of the calls in
>>>> LibvirtComputingResource to pass storage adaptor type instead of defaulting
>>>> to libvirt. That's what I was after.
>>>>
>>>>
>>>> On Mon, Dec 10, 2012 at 2:49 PM, Edison Su <Ed...@citrix.com>wrote:
>>>>
>>>>> One api: ****
>>>>>
>>>>> public KVMStoragePool getStoragePool(String storageAdaptorType, String
>>>>> uuid)****
>>>>>
>>>>> should be enough.****
>>>>>
>>>>> Where the storageAdaptorType coming from? storageAdaptorType should be
>>>>> passed down by mgt server in StorageFilerTO. Whenever you call
>>>>> KVMStoragePoolManager, always pass the storageAdaptorType. In each
>>>>> KVMStoragePoolManager’s api, call getStorageAdaptor at first, find out the
>>>>> corresponding adaptor, then call it.****
>>>>>
>>>>> ** **
>>>>>
>>>>> For example, ****
>>>>>
>>>>>
>>>>> public KVMStoragePool createStoragePool(String name, String host, int port, String path,
>>>>> ****
>>>>>
>>>>> 56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
>>>>> ****
>>>>>
>>>>> ** **
>>>>>
>>>>> may be implemented like this:****
>>>>>
>>>>> storageadaptor adaptor = getstorageadaptor(type);****
>>>>>
>>>>> adaptor.createPool(…)****
>>>>>
>>>>> ** **
>>>>>
>>>>> getStorageAadaptor can be implemented like this:****
>>>>>
>>>>> storageadaptor getStorageAdaptor(StoragePoolType type) {****
>>>>>
>>>>> storageadaptor adaptor = storageMapper.get(type.toString());*
>>>>> ***
>>>>>
>>>>> if (adaptor == null) {****
>>>>>
>>>>> adaptor = storageMapper.get(“libvirt”);****
>>>>>
>>>>> }****
>>>>>
>>>>> return adaptor;****
>>>>>
>>>>> }****
>>>>>
>>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>>> *Sent:* Monday, December 10, 2012 4:25 PM
>>>>>
>>>>> *To:* Edison Su
>>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>>> *Subject:* Re: pluggable storage implementation****
>>>>>
>>>>> ** **
>>>>>
>>>>> I'm not quite sure what you mean on that last part. I was just
>>>>> thinking as far as not having to go through and change all of the existing
>>>>> code to pass 'null' or 'libvirt' when we mean to use libvirt. Also, some of
>>>>> these items in KVMStoragePoolManager (see createStoragePool,
>>>>> createDiskFromTemplate) have code for libvirt specific things (like 'pass
>>>>> this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing with
>>>>> those maybe we keep things working with libvirt as default. ****
>>>>>
>>>>> ** **
>>>>>
>>>>> Something like:****
>>>>>
>>>>> private final Map<String, StorageAdaptor> _storageMapper = new
>>>>> HashMap<String, StorageAdaptor>();****
>>>>>
>>>>> ** **
>>>>>
>>>>> _storageMapper.add("new-storage-adaptor-v1", new
>>>>> MyNewV1StorageAdaptor(storagelayer));****
>>>>>
>>>>> _storageMapper.add("new-storage-adaptor-v2", new
>>>>> MyNewV2StorageAdaptor(storagelayer));****
>>>>>
>>>>> ** **
>>>>>
>>>>> //existing code****
>>>>>
>>>>> public KVMStoragePoolManager(StorageLayer storagelayer,
>>>>> KVMHAMonitor monitor) {****
>>>>>
>>>>> this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);
>>>>> ****
>>>>>
>>>>> this._haMonitor = monitor;****
>>>>>
>>>>> }****
>>>>>
>>>>> ** **
>>>>>
>>>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>>>
>>>>> return this._storageAdaptor.getStoragePool(uuid);****
>>>>>
>>>>> }****
>>>>>
>>>>> ** **
>>>>>
>>>>> // pluggable getStoragePool****
>>>>>
>>>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>>>> String uuid) {****
>>>>>
>>>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>>>
>>>>> ** **
>>>>>
>>>>> return sa.getStoragePool();****
>>>>>
>>>>> }****
>>>>>
>>>>> ** **
>>>>>
>>>>> ** **
>>>>>
>>>>> or alternatively... (but the libvirt specific portions will have to be
>>>>> reworked)****
>>>>>
>>>>> ** **
>>>>>
>>>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>>>
>>>>> return getStoragePool("libvirt", uuid);****
>>>>>
>>>>> }****
>>>>>
>>>>> ** **
>>>>>
>>>>> // pluggable getStoragePool****
>>>>>
>>>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>>>> String uuid) {****
>>>>>
>>>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>>>
>>>>> ** **
>>>>>
>>>>> return sa.getStoragePool();****
>>>>>
>>>>> }****
>>>>>
>>>>> ** **
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> ****
>>>>>
>>>>> ** **
>>>>>
>>>>> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com>
>>>>> wrote:****
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>>> *Sent:* Monday, December 10, 2012 3:37 PM****
>>>>>
>>>>>
>>>>> *To:* Edison Su
>>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>>> *Subject:* Re: pluggable storage implementation****
>>>>>
>>>>> ****
>>>>>
>>>>> Ok, that makes sense. So this will be a part of the storage refactor?
>>>>> If you're not already working on it I'll play with it and see if I can
>>>>> create a working implementation. ****
>>>>>
>>>>> No plan yet, so many stuff on the management server side to refactor.
>>>>> If you will work on it, that’ll be great!****
>>>>>
>>>>> ****
>>>>>
>>>>> I'm assuming we'd have in your example both a getStoragePool(String
>>>>> uuid) that selects the old/default LibvirtStorageAdaptor and a
>>>>> getStoragePool(String storagePoolType, String uuid), so as not to have to
>>>>> redo any existing code?****
>>>>>
>>>>> If you are just implementing a new storage for primary storage, then
>>>>> add a new api called getprimarystorage(string pooltype, string uuid).*
>>>>> ***
>>>>>
>>>>> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com>
>>>>> wrote:****
>>>>>
>>>>> KVMStoragePoolManager should manage the mapping between storage pool
>>>>> type and storageadaptor. ****
>>>>>
>>>>> 1. In KVMStoragePoolManager’s constructor, create a map between
>>>>> storage pool type and storageadaptor:****
>>>>>
>>>>> sample code:****
>>>>>
>>>>> map<String, storageadaptor> storageMapper = new HashMap<String,
>>>>> storageadaptor>();****
>>>>>
>>>>> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>>>>>
>>>>> storageMapper.add(“libvirt-managed-storage”, new
>>>>> LibvirtStorageAdaptor);****
>>>>>
>>>>> 2. For each api of KVMStoragePoolManager, should pass down a storage
>>>>> pool type, e.g:****
>>>>>
>>>>> public KVMStoragePool getStoragePool(String storagePoolType,
>>>>> String uuid) {****
>>>>>
>>>>> storageadaptor adaptor = storageMapper.get(storagepoolType);***
>>>>> *
>>>>>
>>>>> if (adaptor == null) {****
>>>>>
>>>>> adaptor = storageMapper.get(“libvirt-managed-storage”);*
>>>>> ***
>>>>>
>>>>> }****
>>>>>
>>>>> return adaptor.getStoragePool(uuid);****
>>>>>
>>>>> }****
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>>>>>
>>>>> *Sent:* Monday, December 10, 2012 3:06 PM
>>>>> *To:* Edison Su
>>>>> *Cc:* cloudstack-dev@incubator.apache.org****
>>>>>
>>>>> *Subject:* Re: pluggable storage implementation****
>>>>>
>>>>> ****
>>>>>
>>>>> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
>>>>> adaptor, where LibvirtComputingResource relies exclusively on
>>>>> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
>>>>> see how to switch between storage adaptors in KVMStoragePoolManager.java
>>>>> based on what primary storage type I happen to be acting upon.****
>>>>>
>>>>> ****
>>>>>
>>>>> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>
>>>>> wrote:****
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>>> *Sent:* Monday, December 10, 2012 2:06 PM
>>>>> *To:* Edison Su
>>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>>> *Subject:* pluggable storage implementation****
>>>>>
>>>>> ****
>>>>>
>>>>> Just curious, hadn't thought about this before but it seems that at
>>>>> least on KVM (probably similar in Xen and VMware too?), there are two
>>>>> separate issues with storage in the existing code. First, adding a new
>>>>> storage type is a matter of adding in a new 'else if' or something in a
>>>>> bunch of different places, as well as tweaking behavior to match the
>>>>> storage type. Second, everything about the storage is tightly integrated
>>>>> with Libvirt, meaning that if your storage type is not supported by Libvirt
>>>>> it's much, much more difficult to implement.****
>>>>>
>>>>> ****
>>>>>
>>>>> Are these both being addressed by the storage changes, for example can
>>>>> we write a storage plugin that creates pools/volumes that libvirt doesn't
>>>>> know about and still attach those to instances? Or would we need to patch
>>>>> libvirt to utilize our storage first?****
>>>>>
>>>>> ****
>>>>>
>>>>> [Edison] That’s the storageadaptor used for:
>>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
>>>>> ****
>>>>>
>>>>> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
>>>>> which is totally based on libvirt, If you have a new storage, not supported
>>>>> by libvirt, then you can add a new implementation of storageadaptor.**
>>>>> **
>>>>>
>>>>> ****
>>>>>
>>>>> ****
>>>>>
>>>>> ** **
>>>>>
>>>>
>>>>
>>>
>>
>
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
Here's a list according to what's in LibvirtComputingResource of things
that would need to either pass a pool type or a StorageFilerTO:
CreateVolumeFromSnapshotCommand
CreatePrivateTemplateFromVolumeCommand
PrimaryStorageDownloadCommand
Shall I overload these to allow an optional pool type to be passed, so as
not to break or have to rework the other hypervisor types?
On Tue, Dec 11, 2012 at 10:59 AM, Marcus Sorensen <sh...@gmail.com>wrote:
> There are also some commands that don't pass StorageFilerTO, but just a
> uuid of a storage pool (primaryStoragePoolNameLabel), I'm assuming those
> will need to be reworked to pass either a type or the whole StorageFilerTO?
>
>
> On Tue, Dec 11, 2012 at 10:45 AM, Marcus Sorensen <sh...@gmail.com>wrote:
>
>> There are a few snags, there are times when we get the storage pool by
>> asking LibvirtStorageAdaptor, in which case I don't know the adaptor type
>> to pass to KVMStoragePoolManager. For example:
>>
>> secondaryStoragePool =
>> _storagePoolMgr.getStoragePoolByURI(secondaryStorageUrl);
>>
>> In this case it parses the url and then runs createStoragePool on it,
>> returning the resulting pool. It seems these need a separate strategy
>> regarding pulling secondary storage out of Libvirt... do we just hard-code
>> these to libvirt for now, do we need to push the parsing of the url into
>> LibvirtComputingResource, or do you have any suggestions?
>>
>>
>>
>> On Mon, Dec 10, 2012 at 3:27 PM, Marcus Sorensen <sh...@gmail.com>wrote:
>>
>>> Ok, so you do in fact want to change all of the calls in
>>> LibvirtComputingResource to pass storage adaptor type instead of defaulting
>>> to libvirt. That's what I was after.
>>>
>>>
>>> On Mon, Dec 10, 2012 at 2:49 PM, Edison Su <Ed...@citrix.com> wrote:
>>>
>>>> One api: ****
>>>>
>>>> public KVMStoragePool getStoragePool(String storageAdaptorType, String
>>>> uuid)****
>>>>
>>>> should be enough.****
>>>>
>>>> Where the storageAdaptorType coming from? storageAdaptorType should be
>>>> passed down by mgt server in StorageFilerTO. Whenever you call
>>>> KVMStoragePoolManager, always pass the storageAdaptorType. In each
>>>> KVMStoragePoolManager’s api, call getStorageAdaptor at first, find out the
>>>> corresponding adaptor, then call it.****
>>>>
>>>> ** **
>>>>
>>>> For example, ****
>>>>
>>>>
>>>> public KVMStoragePool createStoragePool(String name, String host, int port, String path,
>>>> ****
>>>>
>>>> 56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
>>>> ****
>>>>
>>>> ** **
>>>>
>>>> may be implemented like this:****
>>>>
>>>> storageadaptor adaptor = getstorageadaptor(type);****
>>>>
>>>> adaptor.createPool(…)****
>>>>
>>>> ** **
>>>>
>>>> getStorageAadaptor can be implemented like this:****
>>>>
>>>> storageadaptor getStorageAdaptor(StoragePoolType type) {****
>>>>
>>>> storageadaptor adaptor = storageMapper.get(type.toString());**
>>>> **
>>>>
>>>> if (adaptor == null) {****
>>>>
>>>> adaptor = storageMapper.get(“libvirt”);****
>>>>
>>>> }****
>>>>
>>>> return adaptor;****
>>>>
>>>> }****
>>>>
>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>> *Sent:* Monday, December 10, 2012 4:25 PM
>>>>
>>>> *To:* Edison Su
>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>> *Subject:* Re: pluggable storage implementation****
>>>>
>>>> ** **
>>>>
>>>> I'm not quite sure what you mean on that last part. I was just thinking
>>>> as far as not having to go through and change all of the existing code to
>>>> pass 'null' or 'libvirt' when we mean to use libvirt. Also, some of these
>>>> items in KVMStoragePoolManager (see createStoragePool,
>>>> createDiskFromTemplate) have code for libvirt specific things (like 'pass
>>>> this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing with
>>>> those maybe we keep things working with libvirt as default. ****
>>>>
>>>> ** **
>>>>
>>>> Something like:****
>>>>
>>>> private final Map<String, StorageAdaptor> _storageMapper = new
>>>> HashMap<String, StorageAdaptor>();****
>>>>
>>>> ** **
>>>>
>>>> _storageMapper.add("new-storage-adaptor-v1", new
>>>> MyNewV1StorageAdaptor(storagelayer));****
>>>>
>>>> _storageMapper.add("new-storage-adaptor-v2", new
>>>> MyNewV2StorageAdaptor(storagelayer));****
>>>>
>>>> ** **
>>>>
>>>> //existing code****
>>>>
>>>> public KVMStoragePoolManager(StorageLayer storagelayer,
>>>> KVMHAMonitor monitor) {****
>>>>
>>>> this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);
>>>> ****
>>>>
>>>> this._haMonitor = monitor;****
>>>>
>>>> }****
>>>>
>>>> ** **
>>>>
>>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>>
>>>> return this._storageAdaptor.getStoragePool(uuid);****
>>>>
>>>> }****
>>>>
>>>> ** **
>>>>
>>>> // pluggable getStoragePool****
>>>>
>>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>>> String uuid) {****
>>>>
>>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>>
>>>> ** **
>>>>
>>>> return sa.getStoragePool();****
>>>>
>>>> }****
>>>>
>>>> ** **
>>>>
>>>> ** **
>>>>
>>>> or alternatively... (but the libvirt specific portions will have to be
>>>> reworked)****
>>>>
>>>> ** **
>>>>
>>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>>
>>>> return getStoragePool("libvirt", uuid);****
>>>>
>>>> }****
>>>>
>>>> ** **
>>>>
>>>> // pluggable getStoragePool****
>>>>
>>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>>> String uuid) {****
>>>>
>>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>>
>>>> ** **
>>>>
>>>> return sa.getStoragePool();****
>>>>
>>>> }****
>>>>
>>>> ** **
>>>>
>>>>
>>>>
>>>>
>>>> ****
>>>>
>>>> ** **
>>>>
>>>> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com>
>>>> wrote:****
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>> *Sent:* Monday, December 10, 2012 3:37 PM****
>>>>
>>>>
>>>> *To:* Edison Su
>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>> *Subject:* Re: pluggable storage implementation****
>>>>
>>>> ****
>>>>
>>>> Ok, that makes sense. So this will be a part of the storage refactor?
>>>> If you're not already working on it I'll play with it and see if I can
>>>> create a working implementation. ****
>>>>
>>>> No plan yet, so many stuff on the management server side to refactor.
>>>> If you will work on it, that’ll be great!****
>>>>
>>>> ****
>>>>
>>>> I'm assuming we'd have in your example both a getStoragePool(String
>>>> uuid) that selects the old/default LibvirtStorageAdaptor and a
>>>> getStoragePool(String storagePoolType, String uuid), so as not to have to
>>>> redo any existing code?****
>>>>
>>>> If you are just implementing a new storage for primary storage, then
>>>> add a new api called getprimarystorage(string pooltype, string uuid).**
>>>> **
>>>>
>>>> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com>
>>>> wrote:****
>>>>
>>>> KVMStoragePoolManager should manage the mapping between storage pool
>>>> type and storageadaptor. ****
>>>>
>>>> 1. In KVMStoragePoolManager’s constructor, create a map between
>>>> storage pool type and storageadaptor:****
>>>>
>>>> sample code:****
>>>>
>>>> map<String, storageadaptor> storageMapper = new HashMap<String,
>>>> storageadaptor>();****
>>>>
>>>> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>>>>
>>>> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);
>>>> ****
>>>>
>>>> 2. For each api of KVMStoragePoolManager, should pass down a storage
>>>> pool type, e.g:****
>>>>
>>>> public KVMStoragePool getStoragePool(String storagePoolType,
>>>> String uuid) {****
>>>>
>>>> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>>>>
>>>> if (adaptor == null) {****
>>>>
>>>> adaptor = storageMapper.get(“libvirt-managed-storage”);**
>>>> **
>>>>
>>>> }****
>>>>
>>>> return adaptor.getStoragePool(uuid);****
>>>>
>>>> }****
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>>>>
>>>> *Sent:* Monday, December 10, 2012 3:06 PM
>>>> *To:* Edison Su
>>>> *Cc:* cloudstack-dev@incubator.apache.org****
>>>>
>>>> *Subject:* Re: pluggable storage implementation****
>>>>
>>>> ****
>>>>
>>>> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
>>>> adaptor, where LibvirtComputingResource relies exclusively on
>>>> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
>>>> see how to switch between storage adaptors in KVMStoragePoolManager.java
>>>> based on what primary storage type I happen to be acting upon.****
>>>>
>>>> ****
>>>>
>>>> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>
>>>> wrote:****
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>>> *Sent:* Monday, December 10, 2012 2:06 PM
>>>> *To:* Edison Su
>>>> *Cc:* cloudstack-dev@incubator.apache.org
>>>> *Subject:* pluggable storage implementation****
>>>>
>>>> ****
>>>>
>>>> Just curious, hadn't thought about this before but it seems that at
>>>> least on KVM (probably similar in Xen and VMware too?), there are two
>>>> separate issues with storage in the existing code. First, adding a new
>>>> storage type is a matter of adding in a new 'else if' or something in a
>>>> bunch of different places, as well as tweaking behavior to match the
>>>> storage type. Second, everything about the storage is tightly integrated
>>>> with Libvirt, meaning that if your storage type is not supported by Libvirt
>>>> it's much, much more difficult to implement.****
>>>>
>>>> ****
>>>>
>>>> Are these both being addressed by the storage changes, for example can
>>>> we write a storage plugin that creates pools/volumes that libvirt doesn't
>>>> know about and still attach those to instances? Or would we need to patch
>>>> libvirt to utilize our storage first?****
>>>>
>>>> ****
>>>>
>>>> [Edison] That’s the storageadaptor used for:
>>>> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
>>>> ****
>>>>
>>>> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
>>>> which is totally based on libvirt, If you have a new storage, not supported
>>>> by libvirt, then you can add a new implementation of storageadaptor.***
>>>> *
>>>>
>>>> ****
>>>>
>>>> ****
>>>>
>>>> ** **
>>>>
>>>
>>>
>>
>
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
There are also some commands that don't pass StorageFilerTO, but just a
uuid of a storage pool (primaryStoragePoolNameLabel), I'm assuming those
will need to be reworked to pass either a type or the whole StorageFilerTO?
On Tue, Dec 11, 2012 at 10:45 AM, Marcus Sorensen <sh...@gmail.com>wrote:
> There are a few snags, there are times when we get the storage pool by
> asking LibvirtStorageAdaptor, in which case I don't know the adaptor type
> to pass to KVMStoragePoolManager. For example:
>
> secondaryStoragePool =
> _storagePoolMgr.getStoragePoolByURI(secondaryStorageUrl);
>
> In this case it parses the url and then runs createStoragePool on it,
> returning the resulting pool. It seems these need a separate strategy
> regarding pulling secondary storage out of Libvirt... do we just hard-code
> these to libvirt for now, do we need to push the parsing of the url into
> LibvirtComputingResource, or do you have any suggestions?
>
>
>
> On Mon, Dec 10, 2012 at 3:27 PM, Marcus Sorensen <sh...@gmail.com>wrote:
>
>> Ok, so you do in fact want to change all of the calls in
>> LibvirtComputingResource to pass storage adaptor type instead of defaulting
>> to libvirt. That's what I was after.
>>
>>
>> On Mon, Dec 10, 2012 at 2:49 PM, Edison Su <Ed...@citrix.com> wrote:
>>
>>> One api: ****
>>>
>>> public KVMStoragePool getStoragePool(String storageAdaptorType, String
>>> uuid)****
>>>
>>> should be enough.****
>>>
>>> Where the storageAdaptorType coming from? storageAdaptorType should be
>>> passed down by mgt server in StorageFilerTO. Whenever you call
>>> KVMStoragePoolManager, always pass the storageAdaptorType. In each
>>> KVMStoragePoolManager’s api, call getStorageAdaptor at first, find out the
>>> corresponding adaptor, then call it.****
>>>
>>> ** **
>>>
>>> For example, ****
>>>
>>>
>>> public KVMStoragePool createStoragePool(String name, String host, int port, String path,
>>> ****
>>>
>>> 56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
>>> ****
>>>
>>> ** **
>>>
>>> may be implemented like this:****
>>>
>>> storageadaptor adaptor = getstorageadaptor(type);****
>>>
>>> adaptor.createPool(…)****
>>>
>>> ** **
>>>
>>> getStorageAadaptor can be implemented like this:****
>>>
>>> storageadaptor getStorageAdaptor(StoragePoolType type) {****
>>>
>>> storageadaptor adaptor = storageMapper.get(type.toString());***
>>> *
>>>
>>> if (adaptor == null) {****
>>>
>>> adaptor = storageMapper.get(“libvirt”);****
>>>
>>> }****
>>>
>>> return adaptor;****
>>>
>>> }****
>>>
>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>> *Sent:* Monday, December 10, 2012 4:25 PM
>>>
>>> *To:* Edison Su
>>> *Cc:* cloudstack-dev@incubator.apache.org
>>> *Subject:* Re: pluggable storage implementation****
>>>
>>> ** **
>>>
>>> I'm not quite sure what you mean on that last part. I was just thinking
>>> as far as not having to go through and change all of the existing code to
>>> pass 'null' or 'libvirt' when we mean to use libvirt. Also, some of these
>>> items in KVMStoragePoolManager (see createStoragePool,
>>> createDiskFromTemplate) have code for libvirt specific things (like 'pass
>>> this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing with
>>> those maybe we keep things working with libvirt as default. ****
>>>
>>> ** **
>>>
>>> Something like:****
>>>
>>> private final Map<String, StorageAdaptor> _storageMapper = new
>>> HashMap<String, StorageAdaptor>();****
>>>
>>> ** **
>>>
>>> _storageMapper.add("new-storage-adaptor-v1", new
>>> MyNewV1StorageAdaptor(storagelayer));****
>>>
>>> _storageMapper.add("new-storage-adaptor-v2", new
>>> MyNewV2StorageAdaptor(storagelayer));****
>>>
>>> ** **
>>>
>>> //existing code****
>>>
>>> public KVMStoragePoolManager(StorageLayer storagelayer, KVMHAMonitor
>>> monitor) {****
>>>
>>> this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);*
>>> ***
>>>
>>> this._haMonitor = monitor;****
>>>
>>> }****
>>>
>>> ** **
>>>
>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>
>>> return this._storageAdaptor.getStoragePool(uuid);****
>>>
>>> }****
>>>
>>> ** **
>>>
>>> // pluggable getStoragePool****
>>>
>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>> String uuid) {****
>>>
>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>
>>> ** **
>>>
>>> return sa.getStoragePool();****
>>>
>>> }****
>>>
>>> ** **
>>>
>>> ** **
>>>
>>> or alternatively... (but the libvirt specific portions will have to be
>>> reworked)****
>>>
>>> ** **
>>>
>>> public KVMStoragePool getStoragePool(String uuid) {****
>>>
>>> return getStoragePool("libvirt", uuid);****
>>>
>>> }****
>>>
>>> ** **
>>>
>>> // pluggable getStoragePool****
>>>
>>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>>> String uuid) {****
>>>
>>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>>
>>> ** **
>>>
>>> return sa.getStoragePool();****
>>>
>>> }****
>>>
>>> ** **
>>>
>>>
>>>
>>>
>>> ****
>>>
>>> ** **
>>>
>>> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com> wrote:
>>> ****
>>>
>>> ****
>>>
>>> ****
>>>
>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>> *Sent:* Monday, December 10, 2012 3:37 PM****
>>>
>>>
>>> *To:* Edison Su
>>> *Cc:* cloudstack-dev@incubator.apache.org
>>> *Subject:* Re: pluggable storage implementation****
>>>
>>> ****
>>>
>>> Ok, that makes sense. So this will be a part of the storage refactor? If
>>> you're not already working on it I'll play with it and see if I can create
>>> a working implementation. ****
>>>
>>> No plan yet, so many stuff on the management server side to refactor. If
>>> you will work on it, that’ll be great!****
>>>
>>> ****
>>>
>>> I'm assuming we'd have in your example both a getStoragePool(String
>>> uuid) that selects the old/default LibvirtStorageAdaptor and a
>>> getStoragePool(String storagePoolType, String uuid), so as not to have to
>>> redo any existing code?****
>>>
>>> If you are just implementing a new storage for primary storage, then add
>>> a new api called getprimarystorage(string pooltype, string uuid).****
>>>
>>> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com> wrote:
>>> ****
>>>
>>> KVMStoragePoolManager should manage the mapping between storage pool
>>> type and storageadaptor. ****
>>>
>>> 1. In KVMStoragePoolManager’s constructor, create a map between
>>> storage pool type and storageadaptor:****
>>>
>>> sample code:****
>>>
>>> map<String, storageadaptor> storageMapper = new HashMap<String,
>>> storageadaptor>();****
>>>
>>> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>>>
>>> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);
>>> ****
>>>
>>> 2. For each api of KVMStoragePoolManager, should pass down a storage
>>> pool type, e.g:****
>>>
>>> public KVMStoragePool getStoragePool(String storagePoolType,
>>> String uuid) {****
>>>
>>> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>>>
>>> if (adaptor == null) {****
>>>
>>> adaptor = storageMapper.get(“libvirt-managed-storage”);***
>>> *
>>>
>>> }****
>>>
>>> return adaptor.getStoragePool(uuid);****
>>>
>>> }****
>>>
>>> ****
>>>
>>> ****
>>>
>>> ****
>>>
>>> ****
>>>
>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>>>
>>> *Sent:* Monday, December 10, 2012 3:06 PM
>>> *To:* Edison Su
>>> *Cc:* cloudstack-dev@incubator.apache.org****
>>>
>>> *Subject:* Re: pluggable storage implementation****
>>>
>>> ****
>>>
>>> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
>>> adaptor, where LibvirtComputingResource relies exclusively on
>>> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
>>> see how to switch between storage adaptors in KVMStoragePoolManager.java
>>> based on what primary storage type I happen to be acting upon.****
>>>
>>> ****
>>>
>>> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>
>>> wrote:****
>>>
>>> ****
>>>
>>> ****
>>>
>>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>>> *Sent:* Monday, December 10, 2012 2:06 PM
>>> *To:* Edison Su
>>> *Cc:* cloudstack-dev@incubator.apache.org
>>> *Subject:* pluggable storage implementation****
>>>
>>> ****
>>>
>>> Just curious, hadn't thought about this before but it seems that at
>>> least on KVM (probably similar in Xen and VMware too?), there are two
>>> separate issues with storage in the existing code. First, adding a new
>>> storage type is a matter of adding in a new 'else if' or something in a
>>> bunch of different places, as well as tweaking behavior to match the
>>> storage type. Second, everything about the storage is tightly integrated
>>> with Libvirt, meaning that if your storage type is not supported by Libvirt
>>> it's much, much more difficult to implement.****
>>>
>>> ****
>>>
>>> Are these both being addressed by the storage changes, for example can
>>> we write a storage plugin that creates pools/volumes that libvirt doesn't
>>> know about and still attach those to instances? Or would we need to patch
>>> libvirt to utilize our storage first?****
>>>
>>> ****
>>>
>>> [Edison] That’s the storageadaptor used for:
>>> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
>>> ****
>>>
>>> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
>>> which is totally based on libvirt, If you have a new storage, not supported
>>> by libvirt, then you can add a new implementation of storageadaptor.****
>>>
>>> ****
>>>
>>> ****
>>>
>>> ** **
>>>
>>
>>
>
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
There are a few snags, there are times when we get the storage pool by
asking LibvirtStorageAdaptor, in which case I don't know the adaptor type
to pass to KVMStoragePoolManager. For example:
secondaryStoragePool =
_storagePoolMgr.getStoragePoolByURI(secondaryStorageUrl);
In this case it parses the url and then runs createStoragePool on it,
returning the resulting pool. It seems these need a separate strategy
regarding pulling secondary storage out of Libvirt... do we just hard-code
these to libvirt for now, do we need to push the parsing of the url into
LibvirtComputingResource, or do you have any suggestions?
On Mon, Dec 10, 2012 at 3:27 PM, Marcus Sorensen <sh...@gmail.com>wrote:
> Ok, so you do in fact want to change all of the calls in
> LibvirtComputingResource to pass storage adaptor type instead of defaulting
> to libvirt. That's what I was after.
>
>
> On Mon, Dec 10, 2012 at 2:49 PM, Edison Su <Ed...@citrix.com> wrote:
>
>> One api: ****
>>
>> public KVMStoragePool getStoragePool(String storageAdaptorType, String
>> uuid)****
>>
>> should be enough.****
>>
>> Where the storageAdaptorType coming from? storageAdaptorType should be
>> passed down by mgt server in StorageFilerTO. Whenever you call
>> KVMStoragePoolManager, always pass the storageAdaptorType. In each
>> KVMStoragePoolManager’s api, call getStorageAdaptor at first, find out the
>> corresponding adaptor, then call it.****
>>
>> ** **
>>
>> For example, ****
>>
>>
>> public KVMStoragePool createStoragePool(String name, String host, int port, String path,
>> ****
>>
>> 56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
>> ****
>>
>> ** **
>>
>> may be implemented like this:****
>>
>> storageadaptor adaptor = getstorageadaptor(type);****
>>
>> adaptor.createPool(…)****
>>
>> ** **
>>
>> getStorageAadaptor can be implemented like this:****
>>
>> storageadaptor getStorageAdaptor(StoragePoolType type) {****
>>
>> storageadaptor adaptor = storageMapper.get(type.toString());****
>>
>> if (adaptor == null) {****
>>
>> adaptor = storageMapper.get(“libvirt”);****
>>
>> }****
>>
>> return adaptor;****
>>
>> }****
>>
>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>> *Sent:* Monday, December 10, 2012 4:25 PM
>>
>> *To:* Edison Su
>> *Cc:* cloudstack-dev@incubator.apache.org
>> *Subject:* Re: pluggable storage implementation****
>>
>> ** **
>>
>> I'm not quite sure what you mean on that last part. I was just thinking
>> as far as not having to go through and change all of the existing code to
>> pass 'null' or 'libvirt' when we mean to use libvirt. Also, some of these
>> items in KVMStoragePoolManager (see createStoragePool,
>> createDiskFromTemplate) have code for libvirt specific things (like 'pass
>> this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing with
>> those maybe we keep things working with libvirt as default. ****
>>
>> ** **
>>
>> Something like:****
>>
>> private final Map<String, StorageAdaptor> _storageMapper = new
>> HashMap<String, StorageAdaptor>();****
>>
>> ** **
>>
>> _storageMapper.add("new-storage-adaptor-v1", new
>> MyNewV1StorageAdaptor(storagelayer));****
>>
>> _storageMapper.add("new-storage-adaptor-v2", new
>> MyNewV2StorageAdaptor(storagelayer));****
>>
>> ** **
>>
>> //existing code****
>>
>> public KVMStoragePoolManager(StorageLayer storagelayer, KVMHAMonitor
>> monitor) {****
>>
>> this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);**
>> **
>>
>> this._haMonitor = monitor;****
>>
>> }****
>>
>> ** **
>>
>> public KVMStoragePool getStoragePool(String uuid) {****
>>
>> return this._storageAdaptor.getStoragePool(uuid);****
>>
>> }****
>>
>> ** **
>>
>> // pluggable getStoragePool****
>>
>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>> String uuid) {****
>>
>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>
>> ** **
>>
>> return sa.getStoragePool();****
>>
>> }****
>>
>> ** **
>>
>> ** **
>>
>> or alternatively... (but the libvirt specific portions will have to be
>> reworked)****
>>
>> ** **
>>
>> public KVMStoragePool getStoragePool(String uuid) {****
>>
>> return getStoragePool("libvirt", uuid);****
>>
>> }****
>>
>> ** **
>>
>> // pluggable getStoragePool****
>>
>> public KVMStoragePool getStoragePool(String storageAdaptorType,
>> String uuid) {****
>>
>> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>>
>> ** **
>>
>> return sa.getStoragePool();****
>>
>> }****
>>
>> ** **
>>
>>
>>
>>
>> ****
>>
>> ** **
>>
>> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com> wrote:*
>> ***
>>
>> ****
>>
>> ****
>>
>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>> *Sent:* Monday, December 10, 2012 3:37 PM****
>>
>>
>> *To:* Edison Su
>> *Cc:* cloudstack-dev@incubator.apache.org
>> *Subject:* Re: pluggable storage implementation****
>>
>> ****
>>
>> Ok, that makes sense. So this will be a part of the storage refactor? If
>> you're not already working on it I'll play with it and see if I can create
>> a working implementation. ****
>>
>> No plan yet, so many stuff on the management server side to refactor. If
>> you will work on it, that’ll be great!****
>>
>> ****
>>
>> I'm assuming we'd have in your example both a getStoragePool(String uuid)
>> that selects the old/default LibvirtStorageAdaptor and a
>> getStoragePool(String storagePoolType, String uuid), so as not to have to
>> redo any existing code?****
>>
>> If you are just implementing a new storage for primary storage, then add
>> a new api called getprimarystorage(string pooltype, string uuid).****
>>
>> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com> wrote:*
>> ***
>>
>> KVMStoragePoolManager should manage the mapping between storage pool type
>> and storageadaptor. ****
>>
>> 1. In KVMStoragePoolManager’s constructor, create a map between storage
>> pool type and storageadaptor:****
>>
>> sample code:****
>>
>> map<String, storageadaptor> storageMapper = new HashMap<String,
>> storageadaptor>();****
>>
>> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>>
>> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);*
>> ***
>>
>> 2. For each api of KVMStoragePoolManager, should pass down a storage
>> pool type, e.g:****
>>
>> public KVMStoragePool getStoragePool(String storagePoolType,
>> String uuid) {****
>>
>> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>>
>> if (adaptor == null) {****
>>
>> adaptor = storageMapper.get(“libvirt-managed-storage”);****
>>
>> }****
>>
>> return adaptor.getStoragePool(uuid);****
>>
>> }****
>>
>> ****
>>
>> ****
>>
>> ****
>>
>> ****
>>
>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>>
>> *Sent:* Monday, December 10, 2012 3:06 PM
>> *To:* Edison Su
>> *Cc:* cloudstack-dev@incubator.apache.org****
>>
>> *Subject:* Re: pluggable storage implementation****
>>
>> ****
>>
>> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
>> adaptor, where LibvirtComputingResource relies exclusively on
>> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
>> see how to switch between storage adaptors in KVMStoragePoolManager.java
>> based on what primary storage type I happen to be acting upon.****
>>
>> ****
>>
>> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com> wrote:
>> ****
>>
>> ****
>>
>> ****
>>
>> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
>> *Sent:* Monday, December 10, 2012 2:06 PM
>> *To:* Edison Su
>> *Cc:* cloudstack-dev@incubator.apache.org
>> *Subject:* pluggable storage implementation****
>>
>> ****
>>
>> Just curious, hadn't thought about this before but it seems that at least
>> on KVM (probably similar in Xen and VMware too?), there are two separate
>> issues with storage in the existing code. First, adding a new storage type
>> is a matter of adding in a new 'else if' or something in a bunch of
>> different places, as well as tweaking behavior to match the storage type.
>> Second, everything about the storage is tightly integrated with Libvirt,
>> meaning that if your storage type is not supported by Libvirt it's much,
>> much more difficult to implement.****
>>
>> ****
>>
>> Are these both being addressed by the storage changes, for example can we
>> write a storage plugin that creates pools/volumes that libvirt doesn't know
>> about and still attach those to instances? Or would we need to patch
>> libvirt to utilize our storage first?****
>>
>> ****
>>
>> [Edison] That’s the storageadaptor used for:
>> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
>> ****
>>
>> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
>> which is totally based on libvirt, If you have a new storage, not supported
>> by libvirt, then you can add a new implementation of storageadaptor.****
>>
>> ****
>>
>> ****
>>
>> ** **
>>
>
>
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
Ok, so you do in fact want to change all of the calls in
LibvirtComputingResource to pass storage adaptor type instead of defaulting
to libvirt. That's what I was after.
On Mon, Dec 10, 2012 at 2:49 PM, Edison Su <Ed...@citrix.com> wrote:
> One api: ****
>
> public KVMStoragePool getStoragePool(String storageAdaptorType, String
> uuid)****
>
> should be enough.****
>
> Where the storageAdaptorType coming from? storageAdaptorType should be
> passed down by mgt server in StorageFilerTO. Whenever you call
> KVMStoragePoolManager, always pass the storageAdaptorType. In each
> KVMStoragePoolManager’s api, call getStorageAdaptor at first, find out the
> corresponding adaptor, then call it.****
>
> ** **
>
> For example, ****
>
>
> public KVMStoragePool createStoragePool(String name, String host, int port, String path,
> ****
>
> 56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
> ****
>
> ** **
>
> may be implemented like this:****
>
> storageadaptor adaptor = getstorageadaptor(type);****
>
> adaptor.createPool(…)****
>
> ** **
>
> getStorageAadaptor can be implemented like this:****
>
> storageadaptor getStorageAdaptor(StoragePoolType type) {****
>
> storageadaptor adaptor = storageMapper.get(type.toString());****
>
> if (adaptor == null) {****
>
> adaptor = storageMapper.get(“libvirt”);****
>
> }****
>
> return adaptor;****
>
> }****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 4:25 PM
>
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* Re: pluggable storage implementation****
>
> ** **
>
> I'm not quite sure what you mean on that last part. I was just thinking as
> far as not having to go through and change all of the existing code to pass
> 'null' or 'libvirt' when we mean to use libvirt. Also, some of these items
> in KVMStoragePoolManager (see createStoragePool, createDiskFromTemplate)
> have code for libvirt specific things (like 'pass this to
> LibvirtStorageAdaptor if pool is RBD), so to avoid messing with those maybe
> we keep things working with libvirt as default. ****
>
> ** **
>
> Something like:****
>
> private final Map<String, StorageAdaptor> _storageMapper = new
> HashMap<String, StorageAdaptor>();****
>
> ** **
>
> _storageMapper.add("new-storage-adaptor-v1", new
> MyNewV1StorageAdaptor(storagelayer));****
>
> _storageMapper.add("new-storage-adaptor-v2", new
> MyNewV2StorageAdaptor(storagelayer));****
>
> ** **
>
> //existing code****
>
> public KVMStoragePoolManager(StorageLayer storagelayer, KVMHAMonitor
> monitor) {****
>
> this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);***
> *
>
> this._haMonitor = monitor;****
>
> }****
>
> ** **
>
> public KVMStoragePool getStoragePool(String uuid) {****
>
> return this._storageAdaptor.getStoragePool(uuid);****
>
> }****
>
> ** **
>
> // pluggable getStoragePool****
>
> public KVMStoragePool getStoragePool(String storageAdaptorType, String
> uuid) {****
>
> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>
> ** **
>
> return sa.getStoragePool();****
>
> }****
>
> ** **
>
> ** **
>
> or alternatively... (but the libvirt specific portions will have to be
> reworked)****
>
> ** **
>
> public KVMStoragePool getStoragePool(String uuid) {****
>
> return getStoragePool("libvirt", uuid);****
>
> }****
>
> ** **
>
> // pluggable getStoragePool****
>
> public KVMStoragePool getStoragePool(String storageAdaptorType, String
> uuid) {****
>
> StorageAdaptor sa = storageMapper.get(storageAdaptorType);****
>
> ** **
>
> return sa.getStoragePool();****
>
> }****
>
> ** **
>
>
>
>
> ****
>
> ** **
>
> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com> wrote:**
> **
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 3:37 PM****
>
>
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* Re: pluggable storage implementation****
>
> ****
>
> Ok, that makes sense. So this will be a part of the storage refactor? If
> you're not already working on it I'll play with it and see if I can create
> a working implementation. ****
>
> No plan yet, so many stuff on the management server side to refactor. If
> you will work on it, that’ll be great!****
>
> ****
>
> I'm assuming we'd have in your example both a getStoragePool(String uuid)
> that selects the old/default LibvirtStorageAdaptor and a
> getStoragePool(String storagePoolType, String uuid), so as not to have to
> redo any existing code?****
>
> If you are just implementing a new storage for primary storage, then add a
> new api called getprimarystorage(string pooltype, string uuid).****
>
> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com> wrote:**
> **
>
> KVMStoragePoolManager should manage the mapping between storage pool type
> and storageadaptor. ****
>
> 1. In KVMStoragePoolManager’s constructor, create a map between storage
> pool type and storageadaptor:****
>
> sample code:****
>
> map<String, storageadaptor> storageMapper = new HashMap<String,
> storageadaptor>();****
>
> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>
> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);**
> **
>
> 2. For each api of KVMStoragePoolManager, should pass down a storage
> pool type, e.g:****
>
> public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
> ****
>
> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>
> if (adaptor == null) {****
>
> adaptor = storageMapper.get(“libvirt-managed-storage”);****
>
> }****
>
> return adaptor.getStoragePool(uuid);****
>
> }****
>
> ****
>
> ****
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>
> *Sent:* Monday, December 10, 2012 3:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org****
>
> *Subject:* Re: pluggable storage implementation****
>
> ****
>
> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
> adaptor, where LibvirtComputingResource relies exclusively on
> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
> see how to switch between storage adaptors in KVMStoragePoolManager.java
> based on what primary storage type I happen to be acting upon.****
>
> ****
>
> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com> wrote:*
> ***
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 2:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* pluggable storage implementation****
>
> ****
>
> Just curious, hadn't thought about this before but it seems that at least
> on KVM (probably similar in Xen and VMware too?), there are two separate
> issues with storage in the existing code. First, adding a new storage type
> is a matter of adding in a new 'else if' or something in a bunch of
> different places, as well as tweaking behavior to match the storage type.
> Second, everything about the storage is tightly integrated with Libvirt,
> meaning that if your storage type is not supported by Libvirt it's much,
> much more difficult to implement.****
>
> ****
>
> Are these both being addressed by the storage changes, for example can we
> write a storage plugin that creates pools/volumes that libvirt doesn't know
> about and still attach those to instances? Or would we need to patch
> libvirt to utilize our storage first?****
>
> ****
>
> [Edison] That’s the storageadaptor used for:
> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
> ****
>
> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
> which is totally based on libvirt, If you have a new storage, not supported
> by libvirt, then you can add a new implementation of storageadaptor.****
>
> ****
>
> ****
>
> ** **
>
RE: pluggable storage implementation
Posted by Edison Su <Ed...@citrix.com>.
One api:
public KVMStoragePool getStoragePool(String storageAdaptorType, String uuid)
should be enough.
Where the storageAdaptorType coming from? storageAdaptorType should be passed down by mgt server in StorageFilerTO. Whenever you call KVMStoragePoolManager, always pass the storageAdaptorType. In each KVMStoragePoolManager's api, call getStorageAdaptor at first, find out the corresponding adaptor, then call it.
For example,
public KVMStoragePool createStoragePool(String name, String host, int port, String path,
56<https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStoragePoolManager.java;h=2c0e0ac9b6f5d4e6862cbcfb5a6fd3fe9b5d29d6;hb=HEAD#l56> String userInfo, StoragePoolType type) {
may be implemented like this:
storageadaptor adaptor = getstorageadaptor(type);
adaptor.createPool(...)
getStorageAadaptor can be implemented like this:
storageadaptor getStorageAdaptor(StoragePoolType type) {
storageadaptor adaptor = storageMapper.get(type.toString());
if (adaptor == null) {
adaptor = storageMapper.get("libvirt");
}
return adaptor;
}
From: Marcus Sorensen [mailto:shadowsor@gmail.com]
Sent: Monday, December 10, 2012 4:25 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org
Subject: Re: pluggable storage implementation
I'm not quite sure what you mean on that last part. I was just thinking as far as not having to go through and change all of the existing code to pass 'null' or 'libvirt' when we mean to use libvirt. Also, some of these items in KVMStoragePoolManager (see createStoragePool, createDiskFromTemplate) have code for libvirt specific things (like 'pass this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing with those maybe we keep things working with libvirt as default.
Something like:
private final Map<String, StorageAdaptor> _storageMapper = new HashMap<String, StorageAdaptor>();
_storageMapper.add("new-storage-adaptor-v1", new MyNewV1StorageAdaptor(storagelayer));
_storageMapper.add("new-storage-adaptor-v2", new MyNewV2StorageAdaptor(storagelayer));
//existing code
public KVMStoragePoolManager(StorageLayer storagelayer, KVMHAMonitor monitor) {
this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);
this._haMonitor = monitor;
}
public KVMStoragePool getStoragePool(String uuid) {
return this._storageAdaptor.getStoragePool(uuid);
}
// pluggable getStoragePool
public KVMStoragePool getStoragePool(String storageAdaptorType, String uuid) {
StorageAdaptor sa = storageMapper.get(storageAdaptorType);
return sa.getStoragePool();
}
or alternatively... (but the libvirt specific portions will have to be reworked)
public KVMStoragePool getStoragePool(String uuid) {
return getStoragePool("libvirt", uuid);
}
// pluggable getStoragePool
public KVMStoragePool getStoragePool(String storageAdaptorType, String uuid) {
StorageAdaptor sa = storageMapper.get(storageAdaptorType);
return sa.getStoragePool();
}
On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com>> wrote:
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 3:37 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: Re: pluggable storage implementation
Ok, that makes sense. So this will be a part of the storage refactor? If you're not already working on it I'll play with it and see if I can create a working implementation.
No plan yet, so many stuff on the management server side to refactor. If you will work on it, that'll be great!
I'm assuming we'd have in your example both a getStoragePool(String uuid) that selects the old/default LibvirtStorageAdaptor and a getStoragePool(String storagePoolType, String uuid), so as not to have to redo any existing code?
If you are just implementing a new storage for primary storage, then add a new api called getprimarystorage(string pooltype, string uuid).
On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com>> wrote:
KVMStoragePoolManager should manage the mapping between storage pool type and storageadaptor.
1. In KVMStoragePoolManager's constructor, create a map between storage pool type and storageadaptor:
sample code:
map<String, storageadaptor> storageMapper = new HashMap<String, storageadaptor>();
storageMapper.add("your-storage-type", new your-storage-adaptor);
storageMapper.add("libvirt-managed-storage", new LibvirtStorageAdaptor);
2. For each api of KVMStoragePoolManager, should pass down a storage pool type, e.g:
public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
storageadaptor adaptor = storageMapper.get(storagepoolType);
if (adaptor == null) {
adaptor = storageMapper.get("libvirt-managed-storage");
}
return adaptor.getStoragePool(uuid);
}
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 3:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: Re: pluggable storage implementation
Yeah, but I'm not seeing an easy/pluggable way to implement a storage adaptor, where LibvirtComputingResource relies exclusively on KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't see how to switch between storage adaptors in KVMStoragePoolManager.java based on what primary storage type I happen to be acting upon.
On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>> wrote:
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 2:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: pluggable storage implementation
Just curious, hadn't thought about this before but it seems that at least on KVM (probably similar in Xen and VMware too?), there are two separate issues with storage in the existing code. First, adding a new storage type is a matter of adding in a new 'else if' or something in a bunch of different places, as well as tweaking behavior to match the storage type. Second, everything about the storage is tightly integrated with Libvirt, meaning that if your storage type is not supported by Libvirt it's much, much more difficult to implement.
Are these both being addressed by the storage changes, for example can we write a storage plugin that creates pools/volumes that libvirt doesn't know about and still attach those to instances? Or would we need to patch libvirt to utilize our storage first?
[Edison] That's the storageadaptor used for: https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
The LibvirtStorageAdaptor is one of implementation of storageadaptor, which is totally based on libvirt, If you have a new storage, not supported by libvirt, then you can add a new implementation of storageadaptor.
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
I'm not quite sure what you mean on that last part. I was just thinking as
far as not having to go through and change all of the existing code to pass
'null' or 'libvirt' when we mean to use libvirt. Also, some of these items
in KVMStoragePoolManager (see createStoragePool, createDiskFromTemplate)
have code for libvirt specific things (like 'pass this to
LibvirtStorageAdaptor if pool is RBD), so to avoid messing with those maybe
we keep things working with libvirt as default.
Something like:
private final Map<String, StorageAdaptor> _storageMapper = new
HashMap<String, StorageAdaptor>();
_storageMapper.add("new-storage-adaptor-v1", new
MyNewV1StorageAdaptor(storagelayer));
_storageMapper.add("new-storage-adaptor-v2", new
MyNewV2StorageAdaptor(storagelayer));
//existing code
public KVMStoragePoolManager(StorageLayer storagelayer, KVMHAMonitor
monitor) {
this._storageAdaptor = new LibvirtStorageAdaptor(storagelayer);
this._haMonitor = monitor;
}
public KVMStoragePool getStoragePool(String uuid) {
return this._storageAdaptor.getStoragePool(uuid);
}
// pluggable getStoragePool
public KVMStoragePool getStoragePool(String storageAdaptorType, String
uuid) {
StorageAdaptor sa = storageMapper.get(storageAdaptorType);
return sa.getStoragePool();
}
or alternatively... (but the libvirt specific portions will have to be
reworked)
public KVMStoragePool getStoragePool(String uuid) {
return getStoragePool("libvirt", uuid);
}
// pluggable getStoragePool
public KVMStoragePool getStoragePool(String storageAdaptorType, String
uuid) {
StorageAdaptor sa = storageMapper.get(storageAdaptorType);
return sa.getStoragePool();
}
On Mon, Dec 10, 2012 at 2:05 PM, Edison Su <Ed...@citrix.com> wrote:
> ** **
>
> ** **
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 3:37 PM
>
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* Re: pluggable storage implementation****
>
> ** **
>
> Ok, that makes sense. So this will be a part of the storage refactor? If
> you're not already working on it I'll play with it and see if I can create
> a working implementation. ****
>
> No plan yet, so many stuff on the management server side to refactor. If
> you will work on it, that’ll be great!****
>
> ** **
>
> I'm assuming we'd have in your example both a getStoragePool(String uuid)
> that selects the old/default LibvirtStorageAdaptor and a
> getStoragePool(String storagePoolType, String uuid), so as not to have to
> redo any existing code?****
>
> If you are just implementing a new storage for primary storage, then add a
> new api called getprimarystorage(string pooltype, string uuid).****
>
> On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com> wrote:**
> **
>
> KVMStoragePoolManager should manage the mapping between storage pool type
> and storageadaptor. ****
>
> 1. In KVMStoragePoolManager’s constructor, create a map between storage
> pool type and storageadaptor:****
>
> sample code:****
>
> map<String, storageadaptor> storageMapper = new HashMap<String,
> storageadaptor>();****
>
> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>
> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);**
> **
>
> 2. For each api of KVMStoragePoolManager, should pass down a storage
> pool type, e.g:****
>
> public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
> ****
>
> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>
> if (adaptor == null) {****
>
> adaptor = storageMapper.get(“libvirt-managed-storage”);****
>
> }****
>
> return adaptor.getStoragePool(uuid);****
>
> }****
>
> ****
>
> ****
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com] ****
>
> *Sent:* Monday, December 10, 2012 3:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org****
>
> *Subject:* Re: pluggable storage implementation****
>
> ****
>
> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
> adaptor, where LibvirtComputingResource relies exclusively on
> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
> see how to switch between storage adaptors in KVMStoragePoolManager.java
> based on what primary storage type I happen to be acting upon.****
>
> ****
>
> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com> wrote:*
> ***
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 2:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* pluggable storage implementation****
>
> ****
>
> Just curious, hadn't thought about this before but it seems that at least
> on KVM (probably similar in Xen and VMware too?), there are two separate
> issues with storage in the existing code. First, adding a new storage type
> is a matter of adding in a new 'else if' or something in a bunch of
> different places, as well as tweaking behavior to match the storage type.
> Second, everything about the storage is tightly integrated with Libvirt,
> meaning that if your storage type is not supported by Libvirt it's much,
> much more difficult to implement.****
>
> ****
>
> Are these both being addressed by the storage changes, for example can we
> write a storage plugin that creates pools/volumes that libvirt doesn't know
> about and still attach those to instances? Or would we need to patch
> libvirt to utilize our storage first?****
>
> ****
>
> [Edison] That’s the storageadaptor used for:
> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
> ****
>
> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
> which is totally based on libvirt, If you have a new storage, not supported
> by libvirt, then you can add a new implementation of storageadaptor.****
>
> ****
>
> ** **
>
RE: pluggable storage implementation
Posted by Edison Su <Ed...@citrix.com>.
From: Marcus Sorensen [mailto:shadowsor@gmail.com]
Sent: Monday, December 10, 2012 3:37 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org
Subject: Re: pluggable storage implementation
Ok, that makes sense. So this will be a part of the storage refactor? If you're not already working on it I'll play with it and see if I can create a working implementation.
No plan yet, so many stuff on the management server side to refactor. If you will work on it, that'll be great!
I'm assuming we'd have in your example both a getStoragePool(String uuid) that selects the old/default LibvirtStorageAdaptor and a getStoragePool(String storagePoolType, String uuid), so as not to have to redo any existing code?
If you are just implementing a new storage for primary storage, then add a new api called getprimarystorage(string pooltype, string uuid).
On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com>> wrote:
KVMStoragePoolManager should manage the mapping between storage pool type and storageadaptor.
1. In KVMStoragePoolManager's constructor, create a map between storage pool type and storageadaptor:
sample code:
map<String, storageadaptor> storageMapper = new HashMap<String, storageadaptor>();
storageMapper.add("your-storage-type", new your-storage-adaptor);
storageMapper.add("libvirt-managed-storage", new LibvirtStorageAdaptor);
2. For each api of KVMStoragePoolManager, should pass down a storage pool type, e.g:
public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
storageadaptor adaptor = storageMapper.get(storagepoolType);
if (adaptor == null) {
adaptor = storageMapper.get("libvirt-managed-storage");
}
return adaptor.getStoragePool(uuid);
}
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 3:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: Re: pluggable storage implementation
Yeah, but I'm not seeing an easy/pluggable way to implement a storage adaptor, where LibvirtComputingResource relies exclusively on KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't see how to switch between storage adaptors in KVMStoragePoolManager.java based on what primary storage type I happen to be acting upon.
On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>> wrote:
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 2:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: pluggable storage implementation
Just curious, hadn't thought about this before but it seems that at least on KVM (probably similar in Xen and VMware too?), there are two separate issues with storage in the existing code. First, adding a new storage type is a matter of adding in a new 'else if' or something in a bunch of different places, as well as tweaking behavior to match the storage type. Second, everything about the storage is tightly integrated with Libvirt, meaning that if your storage type is not supported by Libvirt it's much, much more difficult to implement.
Are these both being addressed by the storage changes, for example can we write a storage plugin that creates pools/volumes that libvirt doesn't know about and still attach those to instances? Or would we need to patch libvirt to utilize our storage first?
[Edison] That's the storageadaptor used for: https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
The LibvirtStorageAdaptor is one of implementation of storageadaptor, which is totally based on libvirt, If you have a new storage, not supported by libvirt, then you can add a new implementation of storageadaptor.
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
Ok, that makes sense. So this will be a part of the storage refactor? If
you're not already working on it I'll play with it and see if I can create
a working implementation.
I'm assuming we'd have in your example both a getStoragePool(String uuid)
that selects the old/default LibvirtStorageAdaptor and a
getStoragePool(String storagePoolType, String uuid), so as not to have to
redo any existing code?
On Mon, Dec 10, 2012 at 1:31 PM, Edison Su <Ed...@citrix.com> wrote:
> KVMStoragePoolManager should manage the mapping between storage pool type
> and storageadaptor. ****
>
> **1. **In KVMStoragePoolManager’s constructor, create a map between
> storage pool type and storageadaptor:****
>
> sample code:****
>
> map<String, storageadaptor> storageMapper = new HashMap<String,
> storageadaptor>();****
>
> storageMapper.add(“your-storage-type”, new your-storage-adaptor);****
>
> storageMapper.add(“libvirt-managed-storage”, new LibvirtStorageAdaptor);**
> **
>
> **2. **For each api of KVMStoragePoolManager, should pass down a storage
> pool type, e.g:****
>
> public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
> ****
>
> storageadaptor adaptor = storageMapper.get(storagepoolType);****
>
> if (adaptor == null) {****
>
> adaptor = storageMapper.get(“libvirt-managed-storage”);****
>
> }****
>
> return adaptor.getStoragePool(uuid);****
>
> }****
>
> ** **
>
> ** **
>
> ** **
>
> ** **
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 3:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* Re: pluggable storage implementation****
>
> ** **
>
> Yeah, but I'm not seeing an easy/pluggable way to implement a storage
> adaptor, where LibvirtComputingResource relies exclusively on
> KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
> see how to switch between storage adaptors in KVMStoragePoolManager.java
> based on what primary storage type I happen to be acting upon.****
>
> ** **
>
> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com> wrote:*
> ***
>
> ****
>
> ****
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 2:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* pluggable storage implementation****
>
> ****
>
> Just curious, hadn't thought about this before but it seems that at least
> on KVM (probably similar in Xen and VMware too?), there are two separate
> issues with storage in the existing code. First, adding a new storage type
> is a matter of adding in a new 'else if' or something in a bunch of
> different places, as well as tweaking behavior to match the storage type.
> Second, everything about the storage is tightly integrated with Libvirt,
> meaning that if your storage type is not supported by Libvirt it's much,
> much more difficult to implement.****
>
> ****
>
> Are these both being addressed by the storage changes, for example can we
> write a storage plugin that creates pools/volumes that libvirt doesn't know
> about and still attach those to instances? Or would we need to patch
> libvirt to utilize our storage first?****
>
> ****
>
> [Edison] That’s the storageadaptor used for:
> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
> ****
>
> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
> which is totally based on libvirt, If you have a new storage, not supported
> by libvirt, then you can add a new implementation of storageadaptor.****
>
> ** **
>
RE: pluggable storage implementation
Posted by Edison Su <Ed...@citrix.com>.
KVMStoragePoolManager should manage the mapping between storage pool type and storageadaptor.
1. In KVMStoragePoolManager's constructor, create a map between storage pool type and storageadaptor:
sample code:
map<String, storageadaptor> storageMapper = new HashMap<String, storageadaptor>();
storageMapper.add("your-storage-type", new your-storage-adaptor);
storageMapper.add("libvirt-managed-storage", new LibvirtStorageAdaptor);
2. For each api of KVMStoragePoolManager, should pass down a storage pool type, e.g:
public KVMStoragePool getStoragePool(String storagePoolType, String uuid) {
storageadaptor adaptor = storageMapper.get(storagepoolType);
if (adaptor == null) {
adaptor = storageMapper.get("libvirt-managed-storage");
}
return adaptor.getStoragePool(uuid);
}
From: Marcus Sorensen [mailto:shadowsor@gmail.com]
Sent: Monday, December 10, 2012 3:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org
Subject: Re: pluggable storage implementation
Yeah, but I'm not seeing an easy/pluggable way to implement a storage adaptor, where LibvirtComputingResource relies exclusively on KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't see how to switch between storage adaptors in KVMStoragePoolManager.java based on what primary storage type I happen to be acting upon.
On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com>> wrote:
From: Marcus Sorensen [mailto:shadowsor@gmail.com<ma...@gmail.com>]
Sent: Monday, December 10, 2012 2:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org<ma...@incubator.apache.org>
Subject: pluggable storage implementation
Just curious, hadn't thought about this before but it seems that at least on KVM (probably similar in Xen and VMware too?), there are two separate issues with storage in the existing code. First, adding a new storage type is a matter of adding in a new 'else if' or something in a bunch of different places, as well as tweaking behavior to match the storage type. Second, everything about the storage is tightly integrated with Libvirt, meaning that if your storage type is not supported by Libvirt it's much, much more difficult to implement.
Are these both being addressed by the storage changes, for example can we write a storage plugin that creates pools/volumes that libvirt doesn't know about and still attach those to instances? Or would we need to patch libvirt to utilize our storage first?
[Edison] That's the storageadaptor used for: https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
The LibvirtStorageAdaptor is one of implementation of storageadaptor, which is totally based on libvirt, If you have a new storage, not supported by libvirt, then you can add a new implementation of storageadaptor.
Re: pluggable storage implementation
Posted by Marcus Sorensen <sh...@gmail.com>.
Yeah, but I'm not seeing an easy/pluggable way to implement a storage
adaptor, where LibvirtComputingResource relies exclusively on
KVMStoragePoolManager, which is implementing LibvirtStorageAdaptor. I don't
see how to switch between storage adaptors in KVMStoragePoolManager.java
based on what primary storage type I happen to be acting upon.
On Mon, Dec 10, 2012 at 12:51 PM, Edison Su <Ed...@citrix.com> wrote:
> ** **
>
> ** **
>
> *From:* Marcus Sorensen [mailto:shadowsor@gmail.com]
> *Sent:* Monday, December 10, 2012 2:06 PM
> *To:* Edison Su
> *Cc:* cloudstack-dev@incubator.apache.org
> *Subject:* pluggable storage implementation****
>
> ** **
>
> Just curious, hadn't thought about this before but it seems that at least
> on KVM (probably similar in Xen and VMware too?), there are two separate
> issues with storage in the existing code. First, adding a new storage type
> is a matter of adding in a new 'else if' or something in a bunch of
> different places, as well as tweaking behavior to match the storage type.
> Second, everything about the storage is tightly integrated with Libvirt,
> meaning that if your storage type is not supported by Libvirt it's much,
> much more difficult to implement.****
>
> ** **
>
> Are these both being addressed by the storage changes, for example can we
> write a storage plugin that creates pools/volumes that libvirt doesn't know
> about and still attach those to instances? Or would we need to patch
> libvirt to utilize our storage first?****
>
> ** **
>
> [Edison] That’s the storageadaptor used for:
> https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
> ****
>
> The LibvirtStorageAdaptor is one of implementation of storageadaptor,
> which is totally based on libvirt, If you have a new storage, not supported
> by libvirt, then you can add a new implementation of storageadaptor.****
>
RE: pluggable storage implementation
Posted by Edison Su <Ed...@citrix.com>.
From: Marcus Sorensen [mailto:shadowsor@gmail.com]
Sent: Monday, December 10, 2012 2:06 PM
To: Edison Su
Cc: cloudstack-dev@incubator.apache.org
Subject: pluggable storage implementation
Just curious, hadn't thought about this before but it seems that at least on KVM (probably similar in Xen and VMware too?), there are two separate issues with storage in the existing code. First, adding a new storage type is a matter of adding in a new 'else if' or something in a bunch of different places, as well as tweaking behavior to match the storage type. Second, everything about the storage is tightly integrated with Libvirt, meaning that if your storage type is not supported by Libvirt it's much, much more difficult to implement.
Are these both being addressed by the storage changes, for example can we write a storage plugin that creates pools/volumes that libvirt doesn't know about and still attach those to instances? Or would we need to patch libvirt to utilize our storage first?
[Edison] That's the storageadaptor used for: https://git-wip-us.apache.org/repos/asf?p=incubator-cloudstack.git;a=blob;f=plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/StorageAdaptor.java;h=ef1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=HEAD
The LibvirtStorageAdaptor is one of implementation of storageadaptor, which is totally based on libvirt, If you have a new storage, not supported by libvirt, then you can add a new implementation of storageadaptor.