> On July 17, 2014, 4:55 p.m., Nitin Mehta wrote: > > server/src/com/cloud/vm/UserVmManagerImpl.java, line 960 > > <https://reviews.apache.org/r/23617/diff/1/?file=634289#file634289line960> > > > > Why is it made create=true when it is not a BaseAyncCreate cmd ? > > create=true should be added only when its invoked through the create() > > method of a BaseAyncCreate cmd > > Damodar Reddy Talakanti wrote: > We can not extend AddNicToVMCmd by BaseAsyncCreate cmd as it is doing > many checks before creating the entry NIC. As it is creating entity did made > create=true there.
Many checks is not a problem as long as they are DB checks. Do note we make a command async generally because it is contacting the resource and can take time. If it is just DB checks I would think we can make the command BaseAsyncCreate. Also best that you review with Kishan if create=true will generate all the events (scheduled, started, completed) an async command generates - Nitin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23617/#review48016 ----------------------------------------------------------- On July 17, 2014, 4:43 p.m., Damodar Reddy Talakanti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23617/ > ----------------------------------------------------------- > > (Updated July 17, 2014, 4:43 p.m.) > > > Review request for cloudstack and Nitin Mehta. > > > Repository: cloudstack-git > > > Description > ------- > > Putting the create event into event bus for addNicToVirutalMachine command > explicitly as we can not use BaseAsyncCreateCommand.We can not extend this > with BaseAsyncCreateCmd due to the checks we do before creating the nic. > > > Diffs > ----- > > api/src/com/cloud/event/EventTypes.java 71bfdb6 > server/src/com/cloud/vm/UserVmManagerImpl.java d0bc186 > > Diff: https://reviews.apache.org/r/23617/diff/ > > > Testing > ------- > > Tested against the following setup: > > 1. XenServer 6.2 > 2. Master Branch > > > Thanks, > > Damodar Reddy Talakanti > >
