This is maybe a tangent, but does cloudstack check for uniqueness when generating a UUID? I haven't seen it anywhere, I've always seen it do UUID.randomUUID() and no checking. However, I guess the point is that CS is in control of making sure it's randomly generated, with this patch CS could be fed duplicate UUIDs by a misbehaving program. On Jul 19, 2013 3:23 AM, "Murali Reddy" <murali.re...@citrix.com> wrote:
> Ryan, > > You should open a bug and add details on what the use case you are trying > to solve. Since CloudStack is not in control of UUID generation, you > should at least check that uniqueness of passed in UUID. > > Also please introduce a new smoke test for 'injected job id' instead of > piggy backing the test for deploy VM. > > On 19/07/13 6:02 AM, "Ryan Dietrich" <r...@betterservers.com> wrote: > > > > > > >> On July 18, 2013, 11:34 p.m., Marcus Sorensen wrote: > >> > server/src/com/cloud/async/AsyncJobVO.java, line 141 > >> > > >>< > https://reviews.apache.org/r/12752/diff/1/?file=323558#file323558line141 > >>> > >> > > >> > I'm not immediately certain why this was pulled out. > > > >Nothing is using it. I did a scan to see if anyone is using it, and I > >didn't see anything. Dead could should be removed, right? Git will > >remember for us if we need it later. > > > > > >- Ryan > > > > > >----------------------------------------------------------- > >This is an automatically generated e-mail. To reply, visit: > >https://reviews.apache.org/r/12752/#review23457 > >----------------------------------------------------------- > > > > > >On July 18, 2013, 11:28 p.m., Ryan Dietrich wrote: > >> > >> ----------------------------------------------------------- > >> This is an automatically generated e-mail. To reply, visit: > >> https://reviews.apache.org/r/12752/ > >> ----------------------------------------------------------- > >> > >> (Updated July 18, 2013, 11:28 p.m.) > >> > >> > >> Review request for cloudstack and Marcus Sorensen. > >> > >> > >> Repository: cloudstack-git > >> > >> > >> Description > >> ------- > >> > >> I added "injectedjobid" to the BaseAsyncCmd class as a parameter. > >> If set, it will allow you to tell Cloudstack what the job id instead of > >>it choosing one. > >> A basic string length test is done to verify the variable passed in is > >>actually a UUID. > >> If it is not valid, it is ignored and the job generates it's own. > >> > >> > >> Diffs > >> ----- > >> > >> api/src/org/apache/cloudstack/api/BaseAsyncCmd.java 0e6f95d > >> server/src/com/cloud/api/ApiServer.java 95f17af > >> server/src/com/cloud/async/AsyncJobVO.java 41eccb4 > >> test/integration/smoke/test_deploy_vm.py 425aeb7 > >> tools/marvin/marvin/codegenerator.py 632b8c6 > >> tools/marvin/marvin/integration/lib/base.py 161d03c > >> > >> Diff: https://reviews.apache.org/r/12752/diff/ > >> > >> > >> Testing > >> ------- > >> > >> Updated marvin, updated the deploy vm test. Ran multiple async > >>commands manually, with and without injectedjobid present, no issues > >>detected. > >> > >> > >> Thanks, > >> > >> Ryan Dietrich > >> > >> > > > > > > >
