Hi Kelven, I checked in ecd4a9c6424be73b24a44c73f04c67c47e1611e8.
Creating, deleting, and reverting to VMware snapshots appears to work now, but feel free to try it out of course. :) Talk to you later On Tue, Jan 7, 2014 at 11:31 AM, Mike Tutkowski < mike.tutkow...@solidfire.com> wrote: > Looks like deleting a VMware snapshot was suffering from the same problem > as creating a VMware snapshot. > > I was able to solve both problems the same way. > > Now I'm going to look into reverting to a VMware snapshot. > > > On Tue, Jan 7, 2014 at 10:31 AM, Mike Tutkowski < > mike.tutkow...@solidfire.com> wrote: > >> I logged a bug for this: >> >> >> 1. CLOUDSTACK-5823<https://issues.apache.org/jira/browse/CLOUDSTACK-5823> >> >> >> I assigned this to myself. >> >> I believe I've corrected the issue with taking a VMware snapshot, but I >> want to look into the delete and revert code, as well. >> >> >> >> On Tue, Jan 7, 2014 at 12:20 AM, Mike Tutkowski < >> mike.tutkow...@solidfire.com> wrote: >> >>> If it's true that only the last iteration "counts," couldn't we just >>> grab the last item in this list?: >>> >>> List<Pair<String, ManagedObjectReference>> vmdkFiles = vmMo. >>> getDiskDatastorePathChain(vdisks[i], false); >>> >>> >>> On Tue, Jan 7, 2014 at 12:19 AM, Mike Tutkowski < >>> mike.tutkow...@solidfire.com> wrote: >>> >>>> Hi Kelven, >>>> >>>> I've been playing around with some code to fix this VMware-snapshot >>>> issue. Probably won't have it done until tomorrow, but I wanted to ask you >>>> about this code: >>>> >>>> for (int i = 0; i < vdisks.length; i++) { >>>> >>>> List<Pair<String, ManagedObjectReference>> >>>> vmdkFiles = vmMo.getDiskDatastorePathChain(vdisks[i], false); >>>> >>>> >>>> for (Pair<String, ManagedObjectReference> fileItem >>>> : vmdkFiles) { >>>> >>>> >>>> Can you tell me why we iterate through all of the VMDK files of a >>>> virtual disk? It seems like only the last one "counts." Is that correct? Am >>>> I to assume the last one we iterate over is the most recent snapshot (the >>>> snapshot we just took)? >>>> >>>> Thanks! >>>> >>>> >>>> On Mon, Jan 6, 2014 at 10:47 PM, Mike Tutkowski < >>>> mike.tutkow...@solidfire.com> wrote: >>>> >>>>> Ignore my question about coming up with a baseName. >>>>> >>>>> I see now that volumeTO is not available in the first for loop. >>>>> >>>>> I do think the key and value we have in the map, though, is incorrect. >>>>> >>>>> What do you think? >>>>> >>>>> >>>>> On Mon, Jan 6, 2014 at 10:43 PM, Mike Tutkowski < >>>>> mike.tutkow...@solidfire.com> wrote: >>>>> >>>>>> Hi Kelven, >>>>>> >>>>>> To give you an idea visually what I am referring to, please check out >>>>>> this screen capture: >>>>>> >>>>>> http://i.imgur.com/ma3FE9o.png >>>>>> >>>>>> The key is "i-2" (part of the folder for the VMDK file). >>>>>> >>>>>> The value contains the folder the VMDK file is in. Since the path >>>>>> column for VMware volumes in the DB doesn't contain the folder the VMDK >>>>>> file is in, I think this may be incorrect, as well. >>>>>> >>>>>> I also noticed that we later try to retrieve from the map using >>>>>> volumeTO.getPath() (ignore the getPath() method that enclosing >>>>>> volumeTO.getPath() in the screen shot as this is related to new code...in >>>>>> the standard case, the value of volumeTO.getPath() is just returned from >>>>>> the getPath() method). >>>>>> >>>>>> In the first line of code visible in the screen capture, why do we go >>>>>> to the trouble of doing this: >>>>>> >>>>>> String baseName = extractSnapshotBaseFileName(vmdkName); >>>>>> >>>>>> It seems like this would have worked: >>>>>> >>>>>> String baseName = extractSnapshotBaseFileName(volumTO.getPath()); >>>>>> >>>>>> Or am I missing something there? >>>>>> >>>>>> Thanks!! >>>>>> >>>>>> >>>>>> On Mon, Jan 6, 2014 at 10:13 PM, Mike Tutkowski < >>>>>> mike.tutkow...@solidfire.com> wrote: >>>>>> >>>>>>> Thanks for the info, Kelven. >>>>>>> >>>>>>> I believe we have a serious bug then as null is being assigned to >>>>>>> newPath when a VMware snapshot is being taken (this is in 4.3, by the >>>>>>> way). >>>>>>> >>>>>>> I was trying to fix an issue with VMware snapshots and managed >>>>>>> storage and happened upon this. >>>>>>> >>>>>>> If you have a moment, you might want to set a breakpoint and step >>>>>>> through and see what I mean first hand. >>>>>>> >>>>>>> I'm looking into it, as well. >>>>>>> >>>>>>> Thanks! >>>>>>> >>>>>>> >>>>>>> On Mon, Jan 6, 2014 at 10:02 PM, Kelven Yang <kelven.y...@citrix.com >>>>>>> > wrote: >>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 1/6/14, 5:33 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> >Actually, the more I look at this code, the more I think perhaps >>>>>>>> VMware >>>>>>>> >snapshots are broken because the newPath field should probably not >>>>>>>> be >>>>>>>> >assigned null after creating a new VMware snapshot >>>>>>>> >>>>>>>> >>>>>>>> >(I'm thinking the intend >>>>>>>> >is to replace the other path with a new path that refers to the >>>>>>>> delta file >>>>>>>> >that was just created). >>>>>>>> >>>>>>>> Yes, your guess is correct, the intent to update with a new path is >>>>>>>> to >>>>>>>> reflect the name change after a VM snapshot is taken. When VM >>>>>>>> snapshot is >>>>>>>> involved, one CloudStack volume may have more than one disks be >>>>>>>> related >>>>>>>> with it. So the information in path field only points to the top of >>>>>>>> the >>>>>>>> disk chain, and it is not guaranteed that this information is in >>>>>>>> sync with >>>>>>>> vCenter since there may exist out-of-band changes from vCenter (by >>>>>>>> taking >>>>>>>> VM snapshot in vCenter). >>>>>>>> >>>>>>>> To gracefully work with vCenter, for existing disks that are >>>>>>>> attached to a >>>>>>>> VM, CloudStack only uses the information stored in path field in >>>>>>>> volume >>>>>>>> table as a search basis to connect the record with the real disk >>>>>>>> chain in >>>>>>>> the storage, as soon as it has located the connection, it actually >>>>>>>> uses >>>>>>>> the most recent information from vCenter datastore to setup the disk >>>>>>>> device. In addition, CloudStack also updates the full disk-chain >>>>>>>> information to a field called ³chainInfo². The full chain-info can >>>>>>>> be used >>>>>>>> for recovery/copy-out purpose >>>>>>>> >>>>>>>> Kelven >>>>>>>> >>>>>>>> >>>>>>>> > >>>>>>>> >Does anyone know who worked on VMware snapshots? I've love to ask >>>>>>>> these >>>>>>>> >questions to him soon as we are approaching the end of 4.3. >>>>>>>> > >>>>>>>> >Thanks! >>>>>>>> > >>>>>>>> > >>>>>>>> >On Mon, Jan 6, 2014 at 4:35 PM, Mike Tutkowski >>>>>>>> ><mike.tutkow...@solidfire.com >>>>>>>> >> wrote: >>>>>>>> > >>>>>>>> >> In short, I believe we can remove mapNewDisk and just assign >>>>>>>> null to >>>>>>>> >> newPath. This will keep the existing path for the volume in >>>>>>>> question (in >>>>>>>> >> the volumes table) in the same state as it was before we created >>>>>>>> a >>>>>>>> >>VMware >>>>>>>> >> snapshot, which I believe is the intent anyways. >>>>>>>> >> >>>>>>>> >> Thoughts on that? >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> On Mon, Jan 6, 2014 at 4:10 PM, Mike Tutkowski < >>>>>>>> >> mike.tutkow...@solidfire.com> wrote: >>>>>>>> >> >>>>>>>> >>> Hi, >>>>>>>> >>> >>>>>>>> >>> I was wondering about the following code in >>>>>>>> VmwareStorageManagerImpl. >>>>>>>> >>>It >>>>>>>> >>> is in the CreateVMSnapshotAnswer execute(VmwareHostService >>>>>>>> hostService, >>>>>>>> >>> CreateVMSnapshotCommand cmd) method. >>>>>>>> >>> >>>>>>>> >>> The part I wonder about is in populating the mapNewDisk map. >>>>>>>> For disks >>>>>>>> >>> like the following: >>>>>>>> >>> >>>>>>>> >>> i-2-9-VM/fksjfaklsjdgflajs.vmdk, the key for the map ends up >>>>>>>> being i-2. >>>>>>>> >>> >>>>>>>> >>> When we call this: >>>>>>>> >>> >>>>>>>> >>> String baseName = >>>>>>>> extractSnapshotBaseFileName(volumeTO.getPath()); >>>>>>>> >>> >>>>>>>> >>> It uses a path such as the following: >>>>>>>> >>> >>>>>>>> >>> fksjfaklsjdgflajs >>>>>>>> >>> >>>>>>>> >>> There is no i-2-9-VM/ preceding the name, so the key we search >>>>>>>> on ends >>>>>>>> >>>up >>>>>>>> >>> being the following: >>>>>>>> >>> >>>>>>>> >>> fksjfaklsjdgflajs >>>>>>>> >>> >>>>>>>> >>> This leads to a newPath being equal to null. >>>>>>>> >>> >>>>>>>> >>> As it turns out, I believe null is actually correct, but - if >>>>>>>> that's >>>>>>>> >>>the >>>>>>>> >>> case - why do we have all this logic if - in the end - we are >>>>>>>> just >>>>>>>> >>>going to >>>>>>>> >>> assign null to newPath in every case when creating a VM >>>>>>>> snapshot for >>>>>>>> >>> VMware? As it turns out, null is later interpreted to mean, >>>>>>>> "don't >>>>>>>> >>>replace >>>>>>>> >>> the path field of this volume in the volumes table," which is, >>>>>>>> I think, >>>>>>>> >>> what we want. >>>>>>>> >>> >>>>>>>> >>> Thanks! >>>>>>>> >>> >>>>>>>> >>> VirtualDisk[] vdisks = vmMo.getAllDiskDevice(); >>>>>>>> >>> >>>>>>>> >>> for (int i = 0; i < vdisks.length; i ++){ >>>>>>>> >>> >>>>>>>> >>> List<Pair<String, ManagedObjectReference>> >>>>>>>> >>>vmdkFiles >>>>>>>> >>> = vmMo.getDiskDatastorePathChain(vdisks[i], false); >>>>>>>> >>> >>>>>>>> >>> for(Pair<String, ManagedObjectReference> >>>>>>>> fileItem : >>>>>>>> >>> vmdkFiles) { >>>>>>>> >>> >>>>>>>> >>> String vmdkName = >>>>>>>> fileItem.first().split(" >>>>>>>> >>>")[1]; >>>>>>>> >>> >>>>>>>> >>> if (vmdkName.endsWith(".vmdk")){ >>>>>>>> >>> >>>>>>>> >>> vmdkName = vmdkName.substring(0, >>>>>>>> >>> vmdkName.length() - (".vmdk").length()); >>>>>>>> >>> >>>>>>>> >>> } >>>>>>>> >>> >>>>>>>> >>> String baseName = >>>>>>>> >>> extractSnapshotBaseFileName(vmdkName); >>>>>>>> >>> >>>>>>>> >>> mapNewDisk.put(baseName, vmdkName); >>>>>>>> >>> >>>>>>>> >>> } >>>>>>>> >>> >>>>>>>> >>> } >>>>>>>> >>> >>>>>>>> >>> for (VolumeObjectTO volumeTO : volumeTOs) { >>>>>>>> >>> >>>>>>>> >>> String baseName = >>>>>>>> >>> extractSnapshotBaseFileName(volumeTO.getPath()); >>>>>>>> >>> >>>>>>>> >>> String newPath = mapNewDisk.get(baseName); >>>>>>>> >>> >>>>>>>> >>> // get volume's chain size for this VM >>>>>>>> snapshot, >>>>>>>> >>> exclude current volume vdisk >>>>>>>> >>> >>>>>>>> >>> DataStoreTO store = volumeTO.getDataStore(); >>>>>>>> >>> >>>>>>>> >>> long size = >>>>>>>> >>> getVMSnapshotChainSize(context,hyperHost,baseName + "*.vmdk", >>>>>>>> >>> >>>>>>>> >>> store.getUuid(), newPath); >>>>>>>> >>> >>>>>>>> >>> >>>>>>>> >>> if(volumeTO.getVolumeType()== >>>>>>>> Volume.Type.ROOT){ >>>>>>>> >>> >>>>>>>> >>> // add memory snapshot size >>>>>>>> >>> >>>>>>>> >>> size = size + >>>>>>>> >>> >>>>>>>> getVMSnapshotChainSize(context,hyperHost,cmd.getVmName()+"*.vmsn" >>>>>>>> >>> ,store.getUuid(),null); >>>>>>>> >>> >>>>>>>> >>> } >>>>>>>> >>> >>>>>>>> >>> >>>>>>>> >>> volumeTO.setSize(size); >>>>>>>> >>> >>>>>>>> >>> volumeTO.setPath(newPath); >>>>>>>> >>> >>>>>>>> >>> } >>>>>>>> >>> >>>>>>>> >>> -- >>>>>>>> >>> *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> >>>>>>>> >>> * * >>>>>>>> >>> >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> >>>>>>>> >> -- >>>>>>>> >> *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> >>>>>>>> >> * * >>>>>>>> >> >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> >-- >>>>>>>> >*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> >>>>>>>> >* * >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> *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> >>>>>>> *™* >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> *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> >>>>>> *™* >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> *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> >>>>> *™* >>>>> >>>> >>>> >>>> >>>> -- >>>> *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> >>>> *™* >>>> >>> >>> >>> >>> -- >>> *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> >>> *™* >>> >> >> >> >> -- >> *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> >> *™* >> > > > > -- > *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> > *™* > -- *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> *™*
