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> *™*
