Nice idea to have atomic increment in ResourceLimitManager class. Having it in ResourceLimitManager will solve the resource limit problem for other Resource Types (instances, templates ) and any Resource Types in future.
> -----Original Message----- > From: Chiradeep Vittal [mailto:[email protected]] > Sent: Thursday, July 12, 2012 11:57 PM > To: CloudStack DeveloperList; Nitin Mehta; Deepti Dohare > Subject: Re: Review Request: CS-15430 Create snapshot should fail if creating > snapshot results in exceeding snapshot resource limit for domain-admin or > user accounts. > > Synchronized isn't going to solve all your cases since management servers > can be clustered. > > You need to use GenericDaoBase.acquireInLockTable for these kinds of > synchronization. > > This kind of generic service (atomic increment up to a limit) should be in the > ResourceLimitManager. > Perhaps updateResourceCountForAccount can be modified to take into > account the resource limit. > > -- > Chiradeep > > On 7/11/12 11:02 PM, "Nitin Mehta" <[email protected]> wrote: > > > > >----------------------------------------------------------- > >This is an automatically generated e-mail. To reply, visit: > >https://reviews.apache.org/r/5806/#review9100 > >----------------------------------------------------------- > > > >Ship it! > > > > > >Ship It! > > > >- Nitin Mehta > > > > > >On July 10, 2012, 1:32 p.m., deepti dohare wrote: > >> > >> ----------------------------------------------------------- > >> This is an automatically generated e-mail. To reply, visit: > >> https://reviews.apache.org/r/5806/ > >> ----------------------------------------------------------- > >> > >> (Updated July 10, 2012, 1:32 p.m.) > >> > >> > >> Review request for cloudstack. > >> > >> > >> Description > >> ------- > >> > >> Change: > >> 1. Before creating the snapshot, we synchronized checkResourcelimit > >>to allow the users to create the snapshot and increment the resource > count. > >> 2. Depending on the failure of snapshot creation/ backup, we are > >>decrementing the resource count. > >> > >> > >> This addresses bug CS-15430. > >> > >> > >> Diffs > >> ----- > >> > >> server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java > >>50dcf38 > >> > >> Diff: https://reviews.apache.org/r/5806/diff/ > >> > >> > >> Testing > >> ------- > >> > >> Steps to verify: > >> 1.Login as admin, set snapshot limit '3' for a user account 2.login > >>as user, create a VM1 with data volume 3.trigger 3 create snapshot > >>command from the above data volume, succeeded 4.create one more > >>snapshot, failed, "maximum limit exceeded for account user" > >> > >> > >> Thanks, > >> > >> deepti dohare > >> > >> > >
