Return-Path: X-Original-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Delivered-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 79351D70A for ; Tue, 11 Dec 2012 18:51:37 +0000 (UTC) Received: (qmail 88485 invoked by uid 500); 11 Dec 2012 18:51:37 -0000 Delivered-To: apmail-incubator-cloudstack-dev-archive@incubator.apache.org Received: (qmail 88453 invoked by uid 500); 11 Dec 2012 18:51:37 -0000 Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cloudstack-dev@incubator.apache.org Delivered-To: mailing list cloudstack-dev@incubator.apache.org Received: (qmail 88445 invoked by uid 99); 11 Dec 2012 18:51:37 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 11 Dec 2012 18:51:37 +0000 X-ASF-Spam-Status: No, hits=2.6 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS,TRACKER_ID X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of shadowsor@gmail.com designates 209.85.220.175 as permitted sender) Received: from [209.85.220.175] (HELO mail-vc0-f175.google.com) (209.85.220.175) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 11 Dec 2012 18:51:31 +0000 Received: by mail-vc0-f175.google.com with SMTP id fy7so3885263vcb.6 for ; Tue, 11 Dec 2012 10:51:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=4L2KB/zC9OwJ18L/kJMNKnyRpWkcISFrAR0bgWWdrqY=; b=Af01ovoP8yeu/q1XocmdOvPq+GXKRtbuiaPNXLo8dmYqxYYLUmg348hb4FFoMCAs1e WJFSUa3txoVvMpCCR1+raLfNhRGtlhrbB0xEQGm2a7cVGZXysaL5H2tWFu6iW5t+xod6 xjrO9RbIE6OZLjQDflY38MfzjgDQa6Yz8nCM52rWUmz0wP4SjfHPgjnN4lXXxYGMLrTx y+SgOIXyN6OB1RxbiQwSz10h5h7xzhKXWleidj5ozRdKBvx5W/JZYUDYU/s7tMkHokqk uw/6BDbLV59eoXDtQbKKMRoQv9HI0Jr2ltqmjryEHc5VmazmC/WjcsVDf384mcCeXUA/ pFVA== MIME-Version: 1.0 Received: by 10.52.73.193 with SMTP id n1mr10519918vdv.109.1355251870875; Tue, 11 Dec 2012 10:51:10 -0800 (PST) Received: by 10.58.152.143 with HTTP; Tue, 11 Dec 2012 10:51:10 -0800 (PST) In-Reply-To: References: Date: Tue, 11 Dec 2012 11:51:10 -0700 Message-ID: Subject: Re: pluggable storage implementation From: Marcus Sorensen To: Edison Su Cc: "cloudstack-dev@incubator.apache.org" Content-Type: multipart/alternative; boundary=bcaec5016195583f8f04d0982afe X-Virus-Checked: Checked by ClamAV on apache.org --bcaec5016195583f8f04d0982afe Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable 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 wrot= e: > 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 StorageFilerT= O? > > > On Tue, Dec 11, 2012 at 10:45 AM, Marcus Sorensen wr= ote: > >> 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 typ= e >> to pass to KVMStoragePoolManager. For example: >> >> secondaryStoragePool =3D >> _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-co= de >> 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 wr= ote: >> >>> Ok, so you do in fact want to change all of the calls in >>> LibvirtComputingResource to pass storage adaptor type instead of defaul= ting >>> to libvirt. That's what I was after. >>> >>> >>> On Mon, Dec 10, 2012 at 2:49 PM, Edison Su 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=92s api, call getStorageAdaptor at first, find o= ut the >>>> corresponding adaptor, then call it.**** >>>> >>>> ** ** >>>> >>>> For example, **** >>>> >>>> >>>> public KVMStoragePool createStoragePool(String name, String host, int= port, String path, >>>> **** >>>> >>>> 56 String userInfo, = StoragePoolType type) { >>>> **** >>>> >>>> ** ** >>>> >>>> may be implemented like this:**** >>>> >>>> storageadaptor adaptor =3D getstorageadaptor(type);**** >>>> >>>> adaptor.createPool(=85)**** >>>> >>>> ** ** >>>> >>>> getStorageAadaptor can be implemented like this:**** >>>> >>>> storageadaptor getStorageAdaptor(StoragePoolType type) {**** >>>> >>>> storageadaptor adaptor =3D storageMapper.get(type.toString())= ;** >>>> ** >>>> >>>> if (adaptor =3D=3D null) {**** >>>> >>>> adaptor =3D storageMapper.get(=93libvirt=94);**** >>>> >>>> }**** >>>> >>>> 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 thinkin= g >>>> 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 th= ese >>>> items in KVMStoragePoolManager (see createStoragePool, >>>> createDiskFromTemplate) have code for libvirt specific things (like 'p= ass >>>> this to LibvirtStorageAdaptor if pool is RBD), so to avoid messing wit= h >>>> those maybe we keep things working with libvirt as default. **** >>>> >>>> ** ** >>>> >>>> Something like:**** >>>> >>>> private final Map _storageMapper =3D new >>>> HashMap();**** >>>> >>>> ** ** >>>> >>>> _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 =3D new LibvirtStorageAdaptor(storagelaye= r); >>>> **** >>>> >>>> this._haMonitor =3D monitor;**** >>>> >>>> }**** >>>> >>>> ** ** >>>> >>>> public KVMStoragePool getStoragePool(String uuid) {**** >>>> >>>> return this._storageAdaptor.getStoragePool(uuid);**** >>>> >>>> }**** >>>> >>>> ** ** >>>> >>>> // pluggable getStoragePool**** >>>> >>>> public KVMStoragePool getStoragePool(String storageAdaptorType, >>>> String uuid) {**** >>>> >>>> StorageAdaptor sa =3D 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 =3D storageMapper.get(storageAdaptorType);**= ** >>>> >>>> ** ** >>>> >>>> return sa.getStoragePool();**** >>>> >>>> }**** >>>> >>>> ** ** >>>> >>>> >>>> >>>> >>>> **** >>>> >>>> ** ** >>>> >>>> On Mon, Dec 10, 2012 at 2:05 PM, Edison Su >>>> 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=92ll 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 >>>> wrote:**** >>>> >>>> KVMStoragePoolManager should manage the mapping between storage pool >>>> type and storageadaptor. **** >>>> >>>> 1. In KVMStoragePoolManager=92s constructor, create a map between >>>> storage pool type and storageadaptor:**** >>>> >>>> sample code:**** >>>> >>>> map storageMapper =3D new HashMap>>> storageadaptor>();**** >>>> >>>> storageMapper.add(=93your-storage-type=94, new your-storage-adaptor);*= *** >>>> >>>> storageMapper.add(=93libvirt-managed-storage=94, new LibvirtStorageAda= ptor); >>>> **** >>>> >>>> 2. For each api of KVMStoragePoolManager, should pass down a storage >>>> pool type, e.g:**** >>>> >>>> public KVMStoragePool getStoragePool(String storagePoolType, >>>> String uuid) {**** >>>> >>>> storageadaptor adaptor =3D storageMapper.get(storagepoolType);*= *** >>>> >>>> if (adaptor =3D=3D null) {**** >>>> >>>> adaptor =3D storageMapper.get(=93libvirt-managed-storage= =94);** >>>> ** >>>> >>>> }**** >>>> >>>> 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.ja= va >>>> based on what primary storage type I happen to be acting upon.**** >>>> >>>> **** >>>> >>>> On Mon, Dec 10, 2012 at 12:51 PM, Edison Su >>>> 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 integrat= ed >>>> with Libvirt, meaning that if your storage type is not supported by Li= bvirt >>>> 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 does= n't >>>> know about and still attach those to instances? Or would we need to pa= tch >>>> libvirt to utilize our storage first?**** >>>> >>>> **** >>>> >>>> [Edison] That=92s the storageadaptor used for: >>>> https://git-wip-us.apache.org/repos/asf?p=3Dincubator-cloudstack.git;a= =3Dblob;f=3Dplugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/St= orageAdaptor.java;h=3Def1e7c9302ab6ac8f197cf75bbf7235bba0235cf;hb=3DHEAD >>>> **** >>>> >>>> The LibvirtStorageAdaptor is one of implementation of storageadaptor, >>>> which is totally based on libvirt, If you have a new storage, not supp= orted >>>> by libvirt, then you can add a new implementation of storageadaptor.**= * >>>> * >>>> >>>> **** >>>> >>>> **** >>>> >>>> ** ** >>>> >>> >>> >> > --bcaec5016195583f8f04d0982afe--