Hey Alex, Kelven, I've been looking though the code and thanks for the very insightfull conversation and follow-up email. With merges of this magnitude it's essential to help people understand what is going on.
Purely technical this is a merge i'm really pleased with. It will for sure improve our handling of virtual machines. However the timing of the merge request is not ideal. We are very close to the end of the already extended feature freeze. We have a consensus within the dev community to do large architectural changes in the beginning of the cycle and not at the very end. This not only means a lot of extra testing effort and other developers will have to get used to the changes introduced right at the moment when everybody should be focussed on bug fixing, documentation and stability. Without wanting to rip open old wounds, i can imagine we all want to avoid a javelin incident. I respect the work going into this and the effort it will take to keep this up to date with the current speed of master, but still i'm going to veto this with a -1 for inclusion before we cut of the 4.2 branch. Cheers, Hugo On Jun 26, 2013, at 5:32 PM, Alex Huang <alex.hu...@citrix.com> wrote: > Hi All, > > Kelven had an emergency so I'm submitting the changes on vmsync for him. The > patch are on > https://reviews.apache.org/r/12126. > > Hugo took a look and already had some questions on why so many files were > changed and added/deleted, So I like to explain a bit in this email. > > As part of the vmsync change, we try to move the files that were relevant to > vm orchestration into the right places so that others can properly view the > different jars and understand the relationship between the jars. This > process is ongoing and will continue in other changes. > > - Work related to jobs management have been put under cloud-framework-jobs as > handling jobs is not really part of our server but is a library. By doing it > this way, changes in it can be properly reviewed and there's a good > separation from things that utilizes jobs and jobs itself. > - VirtualMachine orchestration has been moved from cloud-server to > cloud-engine-orchestration. Cloud-engine-orchestration then only depends on > cloud-engine-schema and cloud-api. This creates a clear compilation boundary > between the self-service server implementation and orchestration > implementation. > > As part of these changes, then we surfaced many problems because we setup the > proper compilation boundaries and fixed all of these problems. For example, > HostAllocator interface is something people can write plugins for but it was > buried inside cloud-server package. We've moved a majority of these changes > to cloud-api. There's quite a bit of file movements based on this change. > For vmsync changes itself, the changes are centered around > VirtualMachineManagerImpl.java so you can review that file instead. > > Thanks. > > --Alex > > >> -----Original Message----- >> From: Kelven Yang [mailto:kelven.y...@citrix.com] >> Sent: Tuesday, June 18, 2013 3:38 PM >> To: dev@cloudstack.apache.org >> Subject: Re: [MERGE] Merge VMSync improvement branch into master >> >> >> >> On 6/17/13 7:43 PM, "Mice Xia" <mice_...@tcloudcomputing.com> wrote: >> >>> Kelven, >>> >>> After the refactoring, will CS still restart HA enabled VM when it is >>> power off externally (e.g. using xencenter) or internally (user >>> initiated shutdown or crash)? >> >> >> If HA functionality is provided by external manager (i.e., vCenter), CS won't >> try to restart it, but if HA is provided by CloudStack, we will restore the >> legacy >> logic. The hook up with old HA manager (between >> HighAvailabilityManager/VirtualMachineManager) is in the wrap-up process >> >> >>> >>> Is seems the method HighAvailabilityManagerImpl .scheduleRestart() is >>> not called when VM's actual state is stopped while expected state is >> running. >>> >>> >>> Regards >>> Mice >>> >>> -----Original Message----- >>> From: Kelven Yang [mailto:kelven.y...@citrix.com] >>> Sent: Tuesday, June 18, 2013 5:21 AM >>> To: dev@cloudstack.apache.org >>> Subject: Re: [MERGE] Merge VMSync improvement branch into master >>> >>> Haven't created a patch yet, will do it soon after some last wrap-ups. >>> >>> Kelven >>> >>> On 6/17/13 12:03 PM, "John Burwell" <jburw...@basho.com> wrote: >>> >>>> Kelven, >>>> >>>> Did this patch get pushed to Review Board? If so, what is the URL? >>>> >>>> Thanks. >>>> -John >>>> >>>> On Jun 17, 2013, at 1:40 PM, Kelven Yang <kelven.y...@citrix.com> wrote: >>>> >>>>> Low level classes were tested in unit tests(MessageBus, Job >>>>> framework, Job dispatchers etc), interface layer changes are guarded >>>>> through matching the old semantics, but changes are tested manually, >>>>> we are planning to get this part of testing through BVT system after >>>>> we have re-based the latest master. >>>>> >>>>> Kelven >>>>> >>>>> On 6/17/13 10:01 AM, "Chip Childers" <chip.child...@sungard.com> >> wrote: >>>>> >>>>>> On Mon, Jun 17, 2013 at 04:59:00PM +0000, Kelven Yang wrote: >>>>>>> I'd like to kick off the official merge process. We will start the >>>>>>> merge process after the branch has passed necessary tests >>>>>>> >>>>>>> Kelven >>>>>> >>>>>> Can you share what testing is being run against the branch? >>>>> >>>> >>> >
