> On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote: > > systemvm/scripts/utils.sh, line 9 > > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line9> > > > > Where is the variable CLOUD_COM_HOME defined?
This is a typo. I have fixed this with the new patch file. > On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote: > > systemvm/scripts/utils.sh, line 3 > > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line3> > > > > Where is this variable used? This is a typo. I have fixed this with the new patch file. > On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote: > > systemvm/scripts/run.sh, line 30 > > <https://reviews.apache.org/r/20123/diff/1/?file=552595#file552595line30> > > > > How do we achieve syncrhonization and avoid few race flows here? I mean > > by the time pid value is retrieved at 30, and went to line 32, there could > > be a pid available genuinely for process check we are doing? > > > > As well, if run.sh is called multiple times at same time, the value > > will not be synchronous, then there is no lock protection here? Added locking code via the shells flock. Since run.sh is invoked only via /etc/init.d/cloud it perhaps is not a serious issue. But still to avoid parallel invocations of /etc/init.d/cloud start, locking code has been added. > On April 14, 2014, 8:58 a.m., Santhosh Edukulla wrote: > > systemvm/scripts/utils.sh, line 6 > > <https://reviews.apache.org/r/20123/diff/1/?file=552596#file552596line6> > > > > This is not lock protected and can be run and called multiple times, > > may lead to unwarranted values. The only two scripts which uses get_pids subroutines are the cloud script and the run.sh script and each of them have a local copy of the subroutine as they have sourced utils.sh. which contains the definition. Currently I think there might not be a need to lock protect this one. If you think otherwise, then a little bit more details would be very helpful. - Saurav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20123/#review40251 ----------------------------------------------------------- On April 15, 2014, 11:50 a.m., Saurav Lahiri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20123/ > ----------------------------------------------------------- > > (Updated April 15, 2014, 11:50 a.m.) > > > Review request for cloudstack, Jayapal Reddy, Rajani Karuturi, and Rajesh > Battala. > > > Repository: cloudstack-git > > > Description > ------- > > With multiple java processes writing to the same logfile, each is not aware > of the log4j's internal counter state, this needs to be prevented. So before > starting new java process via the _run.sh , a check is made to ensure that > there are no existing java processes running. This will prevent multiple java > process writing to the same log file namely cloud.out. > > > Diffs > ----- > > systemvm/patches/debian/config/etc/init.d/cloud 83853bc > systemvm/patches/debian/config/etc/init.d/cloud 83853bc > systemvm/scripts/run.sh 146d96f > systemvm/scripts/run.sh 146d96f > systemvm/scripts/utils.sh PRE-CREATION > systemvm/scripts/utils.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/20123/diff/ > > > Testing > ------- > > Tested the changes with console proxy vm and secondary storage vm. They start > and stop as expected. > > > Thanks, > > Saurav Lahiri > >
