Yep, there's a state for snapshotting. That code (from the snippet I provided) appears to be fine.
What drew my attention is the lock on the snapshot object versus a state transition there. On Tuesday, January 13, 2015, Somesh Naidu <somesh.na...@citrix.com> wrote: > Is there a volume state that reflects that it's being snapshotted? If not, > how would mgmt. server know if a subsequent request for snapshot for a > volumes arrives when one is already in progress for that same volume? > > -----Original Message----- > From: Mike Tutkowski [mailto:mike.tutkow...@solidfire.com <javascript:;>] > Sent: Tuesday, January 13, 2015 5:11 PM > To: dev@cloudstack.apache.org <javascript:;> > Subject: Re: [QUESTION] Locking versus State Transitioning > > That is a good question, Nitin. I have not modified that logic. I just copy > and pasted it to e-mail because I was curious about the lock versus state > transition choice on the snapshot. > > I don't think we would need to perform a lock on the volume, though, > because the state transition to "Snapshotting" should prevent code that > honors that state from performing actions against the volume that it > shouldn't when it's in that state. > > I just was mainly wondering why we do a lock against the volume. It seems > that following a state transition to something like "Creating" should do > the trick. > > On Tue, Jan 13, 2015 at 2:21 PM, Nitin Mehta <nitin.me...@citrix.com > <javascript:;>> wrote: > > > Hey Mike, > > State transition shouldn¹t be used for serialization. Our job framework > > should do the same. If not, then its a bug. > > Also, in this case locking on snapshot doesn't help, shouldn¹t it be on > > the volume ? > > > > Thanks, > > -Nitin > > > > On 13/01/15 12:34 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com > <javascript:;>> > > wrote: > > > > >Hi, > > > > > >I noticed while examining the code to take a volume snapshot that > > >XenserverSnapshotStrategy does the following: > > > > > > SnapshotVO snapshotVO = snapshotDao > > >.acquireInLockTable(snapshot.getId()); > > > > > > if (snapshotVO == null) { > > > > > > throw new CloudRuntimeException("Failed to get lock on > > >snapshot:" + snapshot.getId()); > > > > > > } > > > > > > > > > try { > > > > > > VolumeInfo volumeInfo = snapshot.getBaseVolume(); > > > > > > volumeInfo.stateTransit(Volume.Event.SnapshotRequested); > > > > > > SnapshotResult result = null; > > > > > > try { > > > > > > result = snapshotSvr.takeSnapshot(snapshot); > > > > > > if (result.isFailed()) { > > > > > > s_logger.debug("Failed to take snapshot: " + > > >result.getResult()); > > > > > > throw new CloudRuntimeException(result.getResult()); > > > > > > } > > > > > > } finally { > > > > > > if (result != null && result.isSuccess()) { > > > > > > > > >volumeInfo.stateTransit(Volume.Event.OperationSucceeded > > >); > > > > > > } else { > > > > > > > volumeInfo.stateTransit(Volume.Event.OperationFailed); > > > > > > } > > > > > > } > > > > > >Is there a reason here why the code acquires a lock on the snapshot > > >instead > > >of performing a state transition (say, Allocated to Creating)? > > > > > >We can see in the same code block that we use a state transition to > > >protect > > >the volume. > > > > > >Thanks! > > > > > >-- > > >*Mike Tutkowski* > > >*Senior CloudStack Developer, SolidFire Inc.* > > >e: mike.tutkow...@solidfire.com <javascript:;> > > >o: 303.746.7302 > > >Advancing the way the world uses the cloud > > ><http://solidfire.com/solution/overview/?video=play>* * > > > > > > > -- > *Mike Tutkowski* > *Senior CloudStack Developer, SolidFire Inc.* > e: mike.tutkow...@solidfire.com <javascript:;> > o: 303.746.7302 > Advancing the way the world uses the cloud > <http://solidfire.com/solution/overview/?video=play>*™* > -- *Mike Tutkowski* *Senior CloudStack Developer, SolidFire Inc.* e: mike.tutkow...@solidfire.com o: 303.746.7302 Advancing the way the world uses the cloud <http://solidfire.com/solution/overview/?video=play>*™*
