I mention this is my comments on GitHub, as well, but CHAP info is associated with an account - not a storage pool.
Ideally we could do without CHAP info entirely if we had a reliable way to tell the storage plug-in that a given host wants to access a given volume. In this case, my storage plug-in could create what we call a Volume Access Group on the SAN. It would essentially say, "The host with IQN <x> can access the volume with IQN <y> without using CHAP credentials." Of course we'd need a way to revoke this privilege in the event of a live migration of a VM. Right now, I do not believe such a facility is supported with the storage plug-ins. On Thu, Sep 26, 2013 at 4:56 PM, Marcus Sorensen <shadow...@gmail.com>wrote: > Looking at your code, is the chap info stored with the pool, so we > could pass the pool to the adaptor? That would be more agnostic, > anyone implementing a plugin could pull the specifics they need for > their stuff out of the pool on the adaptor side, rather than creating > custom signatures. > > Also, I think we may want to consider implementing connect/disconnect > as just dummy methods in LibvirtStorageAdaptor, so we don't have to be > picky about which adaptors/types in every single place we may want to > connect/disconnect (in 4.1 there were several, I'm not sure if > everything goes through this in 4.2). We can just call > adaptor.connectPhysicalDisk and the adaptor can decide if it needs to > do anything. > > Comments are attached to your commit, I just wanted to echo them here > on-list. > > On Thu, Sep 26, 2013 at 4:32 PM, Mike Tutkowski > <mike.tutkow...@solidfire.com> wrote: > > Oh, SnapshotTestWithFakeData is just modified because the code wasn't > > building until I corrected this. It has nothing really to do with my real > > changes. > > > > > > On Thu, Sep 26, 2013 at 4:31 PM, Mike Tutkowski < > > mike.tutkow...@solidfire.com> wrote: > > > >> Hey Marcus, > >> > >> I implemented your recommendations regarding adding connect and > disconnect > >> methods. It is not yet checked in (as you know, having trouble with my > KVM > >> environment), but it is on GitHub here: > >> > >> > >> > https://github.com/mike-tutkowski/incubator-cloudstack/commit/f2897c65689012e6157c0a0c2ed7e5355900c59a > >> > >> Please let me know if you have any more comments. > >> > >> Thanks! > >> > >> > >> On Thu, Sep 26, 2013 at 4:05 PM, Marcus Sorensen <shadow...@gmail.com > >wrote: > >> > >>> Mike, everyone, > >>> As I've mentioned on the board, I'm working on getting our own > >>> internal KVM storage plugin working on 4.2. In the interest of making > >>> it forward compatible, I just wanted to confirm what you were doing > >>> with the solidfire plugin as far as attaching your iscsi LUNs. We had > >>> discussed a new connectPhysicalDisk method for the StorageAdaptor > >>> class, something perhaps like: > >>> > >>> public boolean connectPhysicalDisk(String volumeUuid, KVMStoragePool > >>> pool); > >>> > >>> then added to KVMStoragePoolManager: > >>> > >>> public boolean connectPhysicalDisk(StoragePoolType type, String > >>> poolUuid, String volPath) { > >>> StorageAdaptor adaptor = getStorageAdaptor(type); > >>> KVMStoragePool pool = adaptor.getStoragePool(poolUuid); > >>> return adaptor.connectPhysicalDisk(volPath, pool); > >>> } > >>> > >>> Something similar to this for disconnect as well. Then in the > >>> KVMStorageProcessor these can be called as needed for attach/detach. > >>> We can probably stub out one in LibvirtStorageAdaptor so there's no > >>> need to switch or if/else for pool types, for example in > >>> KVMStorageProcessor.attachVolume. > >>> > >>> I have debated on whether or not it should just be rolled into > >>> getPhysicalDisk, having it connect the disk if it's not already > >>> connected. getPhysicalDisk is called a lot, and I'm not sure it always > >>> needs to connect the disk when it does. In past iterations > >>> getPhysicalDisk has simply spoken to our SAN api and returned the disk > >>> details, nothing more. So it seemed more flexible and granular to do > >>> the connectPhysicalDisk (we have one now in our 4.1 version). > >>> > >> > >> > >> > >> -- > >> *Mike Tutkowski* > >> *Senior CloudStack Developer, SolidFire Inc.* > >> e: mike.tutkow...@solidfire.com > >> o: 303.746.7302 > >> Advancing the way the world uses the cloud< > http://solidfire.com/solution/overview/?video=play> > >> *™* > >> > > > > > > > > -- > > *Mike Tutkowski* > > *Senior CloudStack Developer, SolidFire Inc.* > > e: mike.tutkow...@solidfire.com > > o: 303.746.7302 > > Advancing the way the world uses the > > cloud<http://solidfire.com/solution/overview/?video=play> > > *™* > -- *Mike Tutkowski* *Senior CloudStack Developer, SolidFire Inc.* e: mike.tutkow...@solidfire.com o: 303.746.7302 Advancing the way the world uses the cloud<http://solidfire.com/solution/overview/?video=play> *™*
