incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Edison Su <Edison...@citrix.com>
Subject RE: re-implement clvm
Date Tue, 07 Aug 2012 23:47:25 GMT
It's been fixed commit: 0ec679c359bfaf8ec43a80fcbc5c617f6c74cab9
Can you get the latest code and rebuild the system iso, copy iso to hypervisor host, then
stop/start ssvm.

> -----Original Message-----
> From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> Sent: Tuesday, August 07, 2012 3:57 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: Re: re-implement clvm
> 
> hacked a fix by editing /var/cache/cloud/cmdline in the ssvm to use
> NfsSecondaryStorageResource. Now I hit
> "http://bugs.cloudstack.org/browse/CS-15143", x != java.lang.String,
> not sure if this is a regression or due to my hack.
> 
> On Tue, Aug 7, 2012 at 4:36 PM, Marcus Sorensen <shadowsor@gmail.com>
> wrote:
> > Edison,
> >   I'm having trouble testing my patch against  incubator-cloudstack.
> > Every indication seems to be that it's working, but the secondary
> > storage VM seems to be messed up. I found this:
> >
> > http://www.mail-archive.com/cloudstack-
> dev@incubator.apache.org/msg02431.html
> >
> > Which you commented on, I just want to ensure that the issues aren't
> > due to my patch. Basically the ssvm doesn't launch its agent. I would
> > like to launch an instance and test my patch further, but am unable
> > to.
> >
> > 22:23:34,035 ERROR AgentShell:606 - Unable to start agent: Resource
> > class not found:
> > com.cloud.storage.resource.PremiumSecondaryStorageResource due to:
> > java.lang.ClassNotFoundException:
> > com.cloud.storage.resource.PremiumSecondaryStorageResource
> >
> > On Mon, Aug 6, 2012 at 2:51 PM, Marcus Sorensen <shadowsor@gmail.com>
> wrote:
> >> Ok, I'll send on the core java patch portion. I've reviewed the
> >> managesnapshot.sh changes. I had someone begin to create a clean
> room
> >> version implementing the same functionality, and in the process
> found
> >> that we can really only change it superficially as well. The clean
> >> room version changed the bulk of the code into a single 'lvcreate
> >> --snapshot' command, however we found that there's an issue with
> >> --snapshot and CLVM exclusive locking, which led the developer to
> >> implement 'lvcreate --snapshot' using equivalent dmsetup commands.
> At
> >> that point we had something that looked the same as the original
> >> patch, with only minor changes in syntax and variable names.
> >>
> >> On Thu, Aug 2, 2012 at 5:11 PM, Edison Su <Edison.su@citrix.com>
> wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> >>>> Sent: Thursday, August 02, 2012 3:09 PM
> >>>> To: cloudstack-dev@incubator.apache.org
> >>>> Subject: RE: re-implement clvm
> >>>>
> >>>> Oh, I understand, but most of the relevant code is implemented in
> >>>> 'if/else
> >>>> if/else' blocks, such that its simply a matter of copying the
> existing
> >>>> RBD
> >>>> code into another 'else if' block and changing a few words (which
> in
> >>>> turn
> >>>> the RBD stuff did previously with the pulled, pre apache CLVM code,
> or
> >>>> so
> >>>> it looks).  In my opinion there's really no other way to do it
> without
> >>>> restructuring to avoid the cascading ifs.
> >>>
> >>>
> >>> That's true, in current code, that's the only way.
> >>>
> >>>>
> >>>> It sounds like aside from the snapshot code the removal was
> probably
> >>>> unnecessary, having been reworked by a third party from Rommer's
> >>>> submissions prior to being merged. I know little about the
> application
> >>>> of
> >>>> licensing details though.
> >>>
> >>> Please send the core java code to reviewboard, we can apply it.
> This part of java code is general enough, meaning everybody wants to
> implement CLVM needs to write the same code.
> >>>
> >>>>
> >>>> What I do know however is that its extremely painful to rip
> something
> >>>> major
> >>>> like this out on existing users, rendering their whole
> infrastructure
> >>>> obsolete with no upgrade path. Especially given the relative
> trivial
> >>>> nature
> >>>> of the patch, you'd think that one of the project owners would
> take an
> >>>> hour
> >>>> or two and rework it. Of course it makes sense that the people who
> use
> >>>> and
> >>>> care about a component should help develop it in an open source
> world,
> >>>> but
> >>>> the cloud stack consumers don't always follow development. Maybe
> we are
> >>>> the
> >>>> only ones who use it, but I think if the next major release pulls
> CLVM
> >>>> support there will be an uproar. What if it had been the code
> >>>> implementing
> >>>> NFS support?
> >>>
> >>> I removed it in May 15, due to the concern that it conflicts with
> Apache license. While at that time, I didn't send an email to dev/user
> list about this decision. That's my mistake. I'll make sure this kind
> of thing will not happen again.
> >>>
> >>>>
> >>>> Sorry for the rant. Hopefully we can get it resolved.
> >>>
> >>> Sure, we can get it resolved.
> >>>
> >>>> On Aug 2, 2012 2:21 PM, "Kevin Kluge" <Kevin.Kluge@citrix.com>
> wrote:
> >>>>
> >>>> > Marcus, you should write the new code in compliance with the
> Apache
> >>>> CLA,
> >>>> > which will forbid directly copying code from some other source.
> >>>> Having
> >>>> > said that, if the problem is constrained enough by existing
> >>>> CloudStack code
> >>>> > and/or the solution is so obvious that your code looks like the
> >>>> original
> >>>> > code, that's just what it is.
> >>>> >
> >>>> > I'm not a lawyer so please don't take this as legal advice from
> >>>> Citrix or
> >>>> > me.
> >>>> >
> >>>> > -kevin
> >>>> >
> >>>> >
> >>>> > > -----Original Message-----
> >>>> > > From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> >>>> > > Sent: Tuesday, July 31, 2012 3:09 PM
> >>>> > > To: cloudstack-dev@incubator.apache.org
> >>>> > > Subject: Re: re-implement clvm
> >>>> > >
> >>>> > > Here's the refactored patch. The CLVM stuff is basically a
> copy of
> >>>> the
> >>>> > RBD
> >>>> > > additions; and the patch also includes the original changes
to
> >>>> > > managesnapshot.sh, which is unmodified.
> >>>> > >
> >>>> > > I'm personally more concerned about the core functionality
at
> this
> >>>> > point, I
> >>>> > > can go through and re-implement the snapshot stuff, but if
the
> core
> >>>> stuff
> >>>> > > can't be pulled in due to licensing then it's not worth the
> trouble
> >>>> to
> >>>> > redo the
> >>>> > > snapshotting. If there's anyone in a position of authority
to
> >>>> address the
> >>>> > > licensing stuff any input would be appreciated.
> >>>> > >
> >>>> > > Thanks,
> >>>> > > Marcus
> >>>> > >
> >>>> > > On Tue, Jul 31, 2012 at 3:21 PM, Edison Su
> <Edison.su@citrix.com>
> >>>> wrote:
> >>>> > > > The most complicated part in rommer's patch is LVM snapshot.
> If
> >>>> > snapshot
> >>>> > > support is not a must, then adding CLVM is simple as RBD.
> >>>> > > >
> >>>> > > >> -----Original Message-----
> >>>> > > >> From: Marcus Sorensen [mailto:shadowsor@gmail.com]
> >>>> > > >> Sent: Tuesday, July 31, 2012 1:33 PM
> >>>> > > >> To: cloudstack-dev@incubator.apache.org
> >>>> > > >> Subject: Re: re-implement clvm
> >>>> > > >>
> >>>> > > >> Ok, so I've created a refactored patch that seems
to work.
> It
> >>>> was
> >>>> > > >> pretty much entirely the RBD additions that were
blocking
> the
> >>>> > > >> original from being rolled back in. If a developer
would be
> >>>> willing
> >>>> > > >> to take on the whole license issue and see this
> functionality
> >>>> put
> >>>> > > >> back in I'd still be willing to pay half of the bounty
> ($400).
> >>>> As
> >>>> > > >> the code looks, the changes are fairly minor, and
I'm not
> sure
> >>>> how
> >>>> > > >> novel you'd have to get avoid the license issues
(or that
> >>>> there's any
> >>>> > > >> easy alternative way to change the code sufficiently)
> >>>> > > >>
> >>>> > > >> On Tue, Jul 31, 2012 at 2:12 PM, David Nalley
> <david@gnsa.us>
> >>>> wrote:
> >>>> > > >> > On Tue, Jul 31, 2012 at 3:56 PM, Wido den Hollander
> >>>> > > >> > <wido@widodh.nl>
> >>>> > > >> wrote:
> >>>> > > >> >>
> >>>> > > >> >>
> >>>> > > >> >> On 07/31/2012 09:48 PM, Marcus Sorensen
wrote:
> >>>> > > >> >>>
> >>>> > > >> >>> I'd be happy to try more if I had access
to any contact
> info.
> >>>> As
> >>>> > > >> it
> >>>> > > >> >>> is, things in the surrounding code have
changed enough
> that
> >>>> a bit
> >>>> > > >> of
> >>>> > > >> >>> re-factoring would need to be done even
if there were
> >>>> permission.
> >>>> > > >> >>>
> >>>> > > >> >>> My hunch is that unless he's switched
roles, once the
> new
> >>>> version
> >>>> > > >> is
> >>>> > > >> >>> released he may come out of the woodwork
wondering why
> that
> >>>> > > thing
> >>>> > > >> he
> >>>> > > >> >>> has a need for and developed is gone.
> >>>> > > >> >>
> >>>> > > >> >>
> >>>> > > >> >> After writing the last RBD implementation
this CLVM
> seems
> >>>> trivial.
> >>>> > > >> >>
> >>>> > > >> >> A lot of code is still in there and looking
at the
> commit
> >>>> where it
> >>>> > > >> got
> >>>> > > >> >> removed it wont be that much work.
> >>>> > > >> >>
> >>>> > > >> >> The problem (and I'm not a licensing expert)
is that if
> I
> >>>> would
> >>>> > > >> implement
> >>>> > > >> >> CLVM again it would look a lot like the
original code,
> do we
> >>>> have
> >>>> > > >> >> to
> >>>> > > >> refer
> >>>> > > >> >> to the old author for that?
> >>>> > > >> >>
> >>>> > > >> >> I'm assuming here that we won't be able
to contact the
> >>>> original
> >>>> > > >> author, but
> >>>> > > >> >> we want to keep the CLVM functionality for
4.0.
> >>>> > > >> >>
> >>>> > > >> >> Wido
> >>>> > > >> >
> >>>> > > >> >
> >>>> > > >> > Actually - you should compare the original patches,
with
> what
> >>>> was
> >>>> > > >> reverted. :
> >>>> > > >> > http://bugs.cloudstack.org/browse/CS-10317
> >>>> > > >> >
> >>>> > > >> > There was already something of a rewrite when
Edison
> changed
> >>>> how
> >>>> > > >> > some of the storage was handled (which is the
iteration
> that
> >>>> was
> >>>> > > pulled).
> >>>> > > >> >
> >>>> > > >> > IANAL either, so I won't bother to even try
and answer
> that
> >>>> > question.
> >>>> > > >> >
> >>>> > > >> > --David
> >>>> >

Mime
View raw message