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 <edison...@citrix.com> wrote:

> ** **
>
> ** **
>
> *From:* Marcus Sorensen [mailto:shadow...@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 <edison...@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:shadow...@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 <edison...@citrix.com> wrote:*
> ***
>
>  ****
>
>  ****
>
> *From:* Marcus Sorensen [mailto:shadow...@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.****
>
>  ****
>
> ** **
>

Reply via email to