Thank Koushik, and I modified what you pointed out (for 'MigrateCommand' text) and updated the diffs.
Let me know if there is anything missing/incorrect. Thanks Alex Ough On Tue, Dec 3, 2013 at 2:46 AM, Koushik Das <koushik....@citrix.com> wrote: > I have posted my review comments. Except for a few minor comments changes > look good to me. > > -Koushik > > On 02-Dec-2013, at 9:15 PM, Alex Ough <alex.o...@sungard.com> wrote: > > > All, > > > > It's been a while since this review was requested, so can anyone review > > this to move on? > > > > Thanks in advance. > > Alex Ough > > > > > > On Fri, Nov 22, 2013 at 11:51 AM, Alex Ough <alex.o...@sungard.com> > wrote: > > > >> Thanks to help from many developers, > >> I sent my first review request in the cloudstack, > >> https://reviews.apache.org/r/15763/, > >> so please take a look at it and let me know if there is anything > >> missing/incorrect. > >> > >> Thanks again. > >> Alex Ough > >> > >> > >> On Thu, Nov 21, 2013 at 10:09 AM, Alex Ough <alex.o...@sungard.com> > wrote: > >> > >>> Hi Wido, > >>> > >>> Related with the 'disk_offering.cache_mode', > >>> I found that there is a sql file to add that column, which is > 'cloudstack/ > >>> developer/target/db/db/schema-430to440.sql'. > >>> But after running it manually, I have another error saying that the > >>> 'cache_mode' cannot be null, so I just changed the column to allow > null to > >>> move on. > >>> > >>> So you may include the sql file and allow null in that column. > >>> > >>> Thanks > >>> Alex Ough > >>> > >>> > >>> On Wed, Nov 20, 2013 at 3:45 PM, Alex Huang <alex.hu...@citrix.com > >wrote: > >>> > >>>> Wido, > >>>> > >>>> > >>>> > >>>> Looks like you didn’t update your schema file or forgot to add a > schema > >>>> upgrade file in master? See the error described by Alex below. > >>>> > >>>> > >>>> > >>>> @Alex > >>>> > >>>> You can probably remove the cachemode field in the DiskOfferingVO for > now. Or revert 1edaa36 on your local repo to keep moving forward. > >>>> > >>>> > >>>> > >>>> --Alex > >>>> > >>>> > >>>> > >>>> *From:* Alex Ough [mailto:alex.o...@sungard.com] > >>>> *Sent:* Wednesday, November 20, 2013 9:56 AM > >>>> *To:* Alex Huang > >>>> *Cc:* dev@cloudstack.apache.org > >>>> > >>>> *Subject:* Re: A question on vm migrations when hosts are set into a > >>>> maintenance mode. > >>>> > >>>> > >>>> > >>>> Hi Alex, > >>>> > >>>> > >>>> > >>>> It looks like you moved the 'ExecuteInSequence' to the vm level from > the > >>>> management server, which seems to be ok, but at this time I cannot > test it > >>>> > >>>> because when I try to start up the management server, it fails because > >>>> of database schema error, which is 'Unknown column > >>>> 'disk_offering.cache_mode' in 'field list' even if I re-built the > database. > >>>> > >>>> > >>>> > >>>> Is the schema change part of your changes or something other developer > >>>> changed? > >>>> > >>>> > >>>> > >>>> If that is from any other developer, can you fix this? > >>>> > >>>> > >>>> > >>>> Thanks > >>>> > >>>> Alex Ough > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>>> On Tue, Nov 19, 2013 at 7:25 PM, Alex Huang <alex.hu...@citrix.com> > >>>> wrote: > >>>> > >>>> Alex, > >>>> > >>>> > >>>> > >>>> Can you do a pull from master and see if my fix fits your needs. > >>>> Unfortunately, I’m on the road and couldn’t do an actual test of it. > >>>> > >>>> > >>>> > >>>> --Alex > >>>> > >>>> > >>>> > >>>> *From:* Alex Huang > >>>> *Sent:* Tuesday, November 19, 2013 4:45 AM > >>>> *To:* 'Alex Ough'; dev@cloudstack.apache.org > >>>> *Subject:* RE: A question on vm migrations when hosts are set into a > >>>> maintenance mode. > >>>> > >>>> > >>>> > >>>> Alex, > >>>> > >>>> > >>>> > >>>> Sorry for the late reply. Been travelling the last couple of weeks. > >>>> I’ll look into this today. > >>>> > >>>> > >>>> > >>>> --Alex > >>>> > >>>> > >>>> > >>>> *From:* Alex Ough [mailto:alex.o...@sungard.com < > alex.o...@sungard.com>] > >>>> > >>>> *Sent:* Monday, November 18, 2013 6:17 AM > >>>> *To:* dev@cloudstack.apache.org > >>>> *Cc:* Alex Huang > >>>> *Subject:* Re: A question on vm migrations when hosts are set into a > >>>> maintenance mode. > >>>> > >>>> > >>>> > >>>> Thank Parasanna & Sebastien, > >>>> > >>>> I also got his email and sent an email. > >>>> > >>>> Waiting for his reply... > >>>> > >>>> > >>>> > >>>> Thanks > >>>> > >>>> Alex Ough > >>>> > >>>> > >>>> > >>>> On Sat, Nov 16, 2013 at 3:05 PM, Sebastien Goasguen <run...@gmail.com > > > >>>> wrote: > >>>> > >>>> cc Alex Huang to get his attention: > >>>> > >>>> > >>>> > >>>> On Nov 15, 2013, at 10:17 PM, Prasanna Santhanam <t...@apache.org> > wrote: > >>>> > >>>>> Alex, Could you just do a git blame on the file and copy the emails > of > >>>>> people who changed that bit of code? They may be able to help if > Cc-ed > >>>>> directly. > >>>>> > >>>>> Thanks, > >>>>> > >>>>> On Fri, Nov 15, 2013 at 01:49:07PM -0600, Alex Ough wrote: > >>>>>> I hate to sending the same emails over and over again, but I really > >>>> need to > >>>>>> finalize this feature to be included in the next code freeze because > >>>> this > >>>>>> feature is very critical in our inside project. > >>>>>> > >>>>>> Anyone who can help, please? > >>>>>> Thanks > >>>>>> Alex Ough > >>>>>> > >>>>>> > >>>>>> On Thu, Nov 14, 2013 at 1:27 PM, Alex Ough <alex.o...@sungard.com> > >>>> wrote: > >>>>>> > >>>>>>> Not sure if Alex Huang checked this, but can anyone help to resolve > >>>> this? > >>>>>>> > >>>>>>> Thanks > >>>>>>> Alex Ough > >>>>>>> > >>>>>>> > >>>>>>> On Wed, Nov 13, 2013 at 11:39 AM, Alex Ough <alex.o...@sungard.com > > > >>>> wrote: > >>>>>>> > >>>>>>>> It sounds a little scary... > >>>>>>>> > >>>>>>>> I looked at the history and found these. > >>>>>>>> > >>>>>>>> 8/9/ : file moved to engine by Alex Huang > >>>>>>>> 9/16 : '_mgmtServer.getExecuteInSequence()' changed to > >>>>>>>> 'getExecuteInSequence()' by Alex Huang > >>>>>>>> > >>>>>>>> > >>>>>>>> Hi Alex Huang, > >>>>>>>> I'm not sure if you're aware of this, but can you check this for > me? > >>>>>>>> > >>>>>>>> Thanks > >>>>>>>> Alex Ough > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On Wed, Nov 13, 2013 at 11:18 AM, Marcus Sorensen < > >>>> shadow...@gmail.com>wrote: > >>>>>>>> > >>>>>>>>> I'm not sure. I know in the past when I've seen files change > >>>> locations > >>>>>>>>> it has also clobbered updates to that file. Someone branched, did > >>>> the > >>>>>>>>> reorganization work, and merged, while in-between the original > file > >>>>>>>>> changed. > >>>>>>>>> > >>>>>>>>> On Wed, Nov 13, 2013 at 9:21 AM, Alex Ough < > alex.o...@sungard.com> > >>>>>>>>> wrote: > >>>>>>>>>> All, > >>>>>>>>>> > >>>>>>>>>> While merging my changes to 4.3 branch, I found that the option, > >>>>>>>>>> 'execute.in.sequence.hypervisor.commands' is NOT used in > >>>>>>>>> Start/Stop/Copy > >>>>>>>>>> commands in 'VirtualMachineManagerImpl.java' any more as below. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> *StopCommand stop = new StopCommand(vm, > getExecuteInSequence());* > >>>>>>>>>> > >>>>>>>>>> *protected boolean getExecuteInSequence() {* > >>>>>>>>>> * return false;* > >>>>>>>>>> *}* > >>>>>>>>>> > >>>>>>>>>> As you see in the above, the function, 'getExecuteInSequence', > >>>> just > >>>>>>>>> returns > >>>>>>>>>> false instead of getting the value from the global variable. > >>>>>>>>>> > >>>>>>>>>> And one more change is that the file has been moved to > >>>>>>>>>> 'engine/orchestration/src/com/cloud/vm' from > >>>> 'server/src/com/cloud/vm'. > >>>>>>>>>> > >>>>>>>>>> Am I missing something related with this or do we stop > supporting > >>>> this > >>>>>>>>>> option in 4.3? > >>>>>>>>>> I'm a little confused, so please help me resolve this. > >>>>>>>>>> > >>>>>>>>>> Thanks > >>>>>>>>>> Alex Ough > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Tue, Nov 12, 2013 at 4:20 PM, Alex Ough < > alex.o...@sungard.com > >>>>> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Thanks a lot for your confirmation, Marcus. > >>>>>>>>>>> I'll create a review request unless anyone has an objection. > >>>>>>>>>>> > >>>>>>>>>>> Thanks > >>>>>>>>>>> Alex Ough > >>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>>> On Tue, Nov 12, 2013 at 3:37 PM, Marcus Sorensen < > >>>> shadow...@gmail.com > >>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> I have done parallel KVM migrations without issue, it's > >>>> "supposed to > >>>>>>>>>>>> work". Really I think it's in the same boat as parallel > >>>> start/stop. > >>>>>>>>> It > >>>>>>>>>>>> should work, but the config option is there just in case. I > >>>> think we > >>>>>>>>>>>> should add it. > >>>>>>>>>>>> > >>>>>>>>>>>> On Thu, Oct 3, 2013 at 11:41 AM, Chip Childers > >>>>>>>>>>>> <chip.child...@sungard.com> wrote: > >>>>>>>>>>>>> On Thu, Oct 03, 2013 at 11:44:46AM -0500, Alex Ough wrote: > >>>>>>>>>>>>>> I'm not sure what else commands 'MigrateCommand' actually > >>>> execute > >>>>>>>>> in > >>>>>>>>>>>>>> addition to 'Start/Stop/CopyCommand', but can we include > >>>>>>>>>>>> 'MigrateCommand' > >>>>>>>>>>>>>> if it consists of only those 3 commands? > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks > >>>>>>>>>>>>>> Alex Ough > >>>>>>>>>>>>> > >>>>>>>>>>>>> In the case of VMware, the migrate command is executed via > the > >>>>>>>>>>>>> MigrateVMTask that's part of the VMware SDK (see > >>>>>>>>>>>>> > >>>>>>>>> > >>>> vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java). > >>>>>>>>>>>>> > >>>>>>>>>>>>> For VMware, I know that vCenter will queue and process > >>>> concurrent > >>>>>>>>>>>>> requests for migrations. Specifically, it will throttle the > >>>>>>>>> migrations > >>>>>>>>>>>>> happening, based on it's internal concurrency constraints, > but > >>>> the > >>>>>>>>> task > >>>>>>>>>>>>> queue will still accept more connections. Obviously the risk > >>>> are > >>>>>>>>> the > >>>>>>>>>>>>> VMware layer tasks timing out if it takes too long for the > task > >>>>>>>>> queue to > >>>>>>>>>>>>> complete. > >>>>>>>>>>>>> > >>>>>>>>>>>>> As for XenServer, it's happening in what appears to be a > >>>> similar > >>>>>>>>> way > >>>>>>>>>>>>> (although the source host is the target for the migration API > >>>>>>>>> call). > >>>>>>>>>>>>> > >>>>>>>>>>>>> Check > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>> > >>>> > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I'm not familiar enough with XenServer's concurrency model > for > >>>>>>>>>>>>> migrations. Any experts know the answer to if it can handle > >>>>>>>>> concurrency > >>>>>>>>>>>>> in a stable way? > >>>>>>>>>>>>> > >>>>>>>>>>>>> With KVM, it's obviously executing via the agent. Similarly > to > >>>>>>>>>>>>> XenServer, I'm not familiar enough to know about concurrent > >>>>>>>>> operations. > >>>>>>>>>>>>> > >>>>>>>>>>>>> So do the HV experts on the list have any opinions about > >>>> XenServer > >>>>>>>>> and > >>>>>>>>>>>>> KVM migration concurrency? > >>>>>>>>>>>>> > >>>>>>>>>>>>> -chip > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>> > >>>>> -- > >>>>> Prasanna., > >>>>> > >>>>> ------------------------ > >>>>> Powered by BigRock.com > >>>>> > >>>> > >>>> > >>>> > >>>> > >>>> > >>> > >>> > >> > > >
