cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Tutkowski <mike.tutkow...@solidfire.com>
Subject Re: add connect method on KVM storage code
Date Fri, 27 Sep 2013 01:32:34 GMT
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 <shadowsor@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.tutkowski@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.tutkowski@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 <shadowsor@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.tutkowski@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.tutkowski@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.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message