Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (athena.apache.org: domain of mike.tutkowski@solidfire.com
 designates 209.85.128.176 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CALFpzo5yNZ-y6USA=L0ts1EPcWD7oo9fHQjxFVDgifm1WqrfWA@mail.gmail.com>
References: 
 <CALFpzo60k1yNhXXc6Kp9w7vZD5T81Uq3WWDL-g7gx4gUYzWNZQ@mail.gmail.com>
	<CACOnxCifYvMw5DS7Eq-vtK6iXmHMWyVH5faZqPBQUCfoXcpKog@mail.gmail.com>
	<CACOnxChoxUNzpDtBt=N-U7jkGBY-YT8-L7g7HosEhn7zaLsYNw@mail.gmail.com>
	<CALFpzo5yNZ-y6USA=L0ts1EPcWD7oo9fHQjxFVDgifm1WqrfWA@mail.gmail.com>
Date: Thu, 26 Sep 2013 19:32:34 -0600
Message-ID: 
 <CACOnxCi8BAVVzJ5Fza26twMs_rd=xE7uuQ0pNCr70dai09cM4g@mail.gmail.com>
Subject: Re: add connect method on KVM storage code
From: Mike Tutkowski <mike.tutkowski@solidfire.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
Content-Type: multipart/alternative; boundary=e89a8f6474edfaf82404e7537532

--e89a8f6474edfaf82404e7537532
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable

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 re=
al
> > 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/f2897c65689=
012e6157c0a0c2ed7e5355900c59a
> >>
> >> 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 =3D getStorageAdaptor(type);
> >>>         KVMStoragePool pool =3D 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 alway=
s
> >>> needs to connect the disk when it does. In past iterations
> >>> getPhysicalDisk has simply spoken to our SAN api and returned the dis=
k
> >>> 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=3Dplay>
> >> *=99*
> >>
> >
> >
> >
> > --
> > *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=3Dplay>
> > *=99*
>


--=20
*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=3Dplay>
*=99*

--e89a8f6474edfaf82404e7537532--