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