Mike, Thanks for digging into the details, I’ll check it out after I’m done with my current things in hand
Kelven On 1/7/14, 11:50 AM, "Mike Tutkowski" <mike.tutkow...@solidfire.com> wrote: >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> >*™*
