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 <[email protected]> wrote: > > > On 1/6/14, 5:33 PM, "Mike Tutkowski" <[email protected]> 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 > ><[email protected] > >> 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 < > >> [email protected]> 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: [email protected] > >>> 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: [email protected] > >> 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: [email protected] > >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: [email protected] o: 303.746.7302 Advancing the way the world uses the cloud<http://solidfire.com/solution/overview/?video=play> *™*
